Re: Fix handling of invalid sockets returned by PQsocket()

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix handling of invalid sockets returned by PQsocket()
Date: 2016-03-07 13:10:33
Message-ID: CAB7nPqRE5_onCjNE9SQuthqxqFrdUHSZpOrh=Jw1uanydu5XqA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Mar 6, 2016 at 12:52 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Peter Eisentraut wrote:
>> On 2/17/16 10:52 PM, Michael Paquier wrote:
>> > On Thu, Feb 18, 2016 at 1:58 AM, Alvaro Herrera
>> > <alvherre(at)2ndquadrant(dot)com> wrote:
>> >> Michael Paquier wrote:
>> >>> Hi all,
>> >>>
>> >>> After looking at Alvaro's message mentioning the handling of
>> >>> PQsocket() for invalid sockets, I just had a look by curiosity at
>> >>> other calls of this routine, and found a couple of issues:
>> >>> 1) In vacuumdb.c, init_slot() does not check for the return value of PQsocket():
>> >>> slot->sock = PQsocket(conn);
>> >>> 2) In isolationtester.c, try_complete_step() should do the same.
>> >>> 3) In pg_recvlogical.c for StreamLogicalLog() I am spotting the same problem.
>> >>> I guess those ones should be fixed as well, no?
>> >>
>> >> I patched pgbench to use PQerrorMessage rather than strerror(errno). I
>> >> think your patch should do the same.
>> >
>> > OK, this looks like a good idea. I would suggest doing the same in
>> > receivelog.c then.
>>
>> Let's make the error messages consistent as "invalid socket". "bad
>> socket" isn't really our style, and pg_basebackup saying "socket not
>> open" is just plain incorrect.
>
> You're completely right. Let's patch pgbench that way too.

Here is v3 then, switching to "invalid socket" for those error
messages. There are two extra messages in fe-misc.c and
libpqwalreceiver.c that need a rewording that I have detected as well.
--
Michael

Attachment Content-Type Size
pqsocket-error-fix-v3.patch application/x-patch 4.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Steele 2016-03-07 13:36:04 Re: (pgaudit) Audit log is not output after the SET ROLE.
Previous Message MauMau 2016-03-07 12:55:07 Re: How can we expand PostgreSQL ecosystem?