Re: [HACKERS][PATCH] adding simple sock check for windows - Mailing list pgsql-hackers

From CharSyam
Subject Re: [HACKERS][PATCH] adding simple sock check for windows
Date
Msg-id CAMrLSE72q0ca2+Tf3V7-74NB-TeuSWh+0gLE5xdpEAxFt7JphQ@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS][PATCH] adding simple sock check for windows  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Hi, Tom, Thank you for your review.
so Do you think it is better to remove if statement like below

diff --git src/bin/scripts/vacuumdb.c src/bin/scripts/vacuumdb.c
index 887fa48fbd..243d842d06 100644
--- src/bin/scripts/vacuumdb.c
+++ src/bin/scripts/vacuumdb.c
@@ -947,13 +947,6 @@ init_slot(ParallelSlot *slot, PGconn *conn, const
char *progname)
     slot->connection = conn;
     slot->isFree = true;
     slot->sock = PQsocket(conn);
-
-    if (slot->sock < 0)
-    {
-        fprintf(stderr, _("%s: invalid socket: %s"), progname,
-                PQerrorMessage(conn));
-        exit(1);
-    }
 }

 static void

2018-04-01 2:16 GMT+09:00 Tom Lane <tgl@sss.pgh.pa.us>:
> CharSyam <charsyam@gmail.com> writes:
>> [ simple_check.patch ]
>
> This is a good catch.  However, it looks to me like the reason nobody
> has noticed a problem here is that actually, this error test is useless;
> we can never get here with a bad connection, because connectDatabase
> would have failed.  I'm inclined to think we should just delete the whole
> if-statement, instead.
>
> BTW, I tried cross-checking for other errors of this ilk by doing
>
> diff --git a/src/include/port.h b/src/include/port.h
> index a514ab758b..9ba6a0df79 100644
> --- a/src/include/port.h
> +++ b/src/include/port.h
> @@ -28,9 +28,9 @@
>
>  /* socket has a different definition on WIN32 */
>  #ifndef WIN32
> -typedef int pgsocket;
> +typedef unsigned int pgsocket;
>
> -#define PGINVALID_SOCKET (-1)
> +#define PGINVALID_SOCKET ((pgsocket) -1)
>  #else
>  typedef SOCKET pgsocket;
>
> and then compiling with a compiler that would warn about "unsigned int < 0"
> tests (I used Apple's current compiler, but probably any recent clang
> would do as well).  It found this case and no others, which is good.
> This is not a 100% cross-check, because I don't think it would notice
> "unsigned int == -1" which is another way somebody might misspell the
> test, and it definitely would not catch cases where somebody stored a
> socket identifier in plain int instead of pgsocket.  But it's better
> than nothing...
>
>                         regards, tom lane

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [HACKERS][PATCH] adding simple sock check for windows
Next
From: Pavel Stehule
Date:
Subject: some last patches breaks plan cache