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

Reply via email to