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 <t...@sss.pgh.pa.us>:
> CharSyam <chars...@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
remove_if_stat.patch
Description: Binary data
