Set Neon to return meaningful socket errors on Windows during socket.recv() calls. #445

gitlarryf · 2024-01-22T21:28:34Z

Fix net.neon to return meaningful errors on recv()

ghewgill · 2024-01-23T21:01:46Z

src/socketx.h

@@ -3,6 +3,7 @@
 #include <winsock.h>
 typedef int socklen_t;
 #pragma warning(disable: 4127) // incompatible with FD_SET()
+#define socketerror  WSAGetLastError()


Use #define socketerror() so the use of this doesn't look like a variable.

This has not been fixed.

ghewgill · 2024-01-23T21:01:53Z

src/socketx.h

@@ -15,5 +16,6 @@ typedef int socklen_t;
 typedef int SOCKET;
 const int INVALID_SOCKET = -1;
 inline void closesocket(int x) { close(x); }
+#define socketerror   errno


Use #define socketerror() so the use of this doesn't look like a variable.

Also this has not been fixed.

ghewgill · 2024-01-23T21:02:55Z

lib/net.cpp

+                    sizeof(msgbuf),
+                    NULL);
+#else
+    snprintf(msgbuf, sizeof(msgbuf), "%s", strerror(err));


To avoid an unnecessary copy, this case can be simply return strerror(err). You should be aware of default constructors and how it is unnecessary to explicitly call std::string(...) for character buffers.

ghewgill · 2024-01-23T21:05:16Z

lib/net.cpp

+{
+    static char msgbuf[256] = { '\0' };
+#ifdef _WIN32
+    FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,


The return value of FormatMessage is not checked here, and it should be.

ghewgill · 2024-01-23T21:06:31Z

lib/net.cpp

+#else
+    snprintf(msgbuf, sizeof(msgbuf), "%s", strerror(err));
+#endif
+    if (msgbuf[0] == '\0') {


This seems like a redundant test, as I can see no case where the error message function succeeds but still returns an empty message. Instead, check for success of FormatMessage. (Note that strerror() has no error return value and is documented to always return a valid string.)

Removed unnecessary check, and simplified the logic.

ghewgill · 2024-02-02T21:38:13Z

src/socketx.h

@@ -3,6 +3,7 @@
 #include <winsock.h>
 typedef int socklen_t;
 #pragma warning(disable: 4127) // incompatible with FD_SET()
+#define socketerror  WSAGetLastError()


This has not been fixed.

ghewgill · 2024-02-02T21:38:31Z

src/socketx.h

@@ -15,5 +16,6 @@ typedef int socklen_t;
 typedef int SOCKET;
 const int INVALID_SOCKET = -1;
 inline void closesocket(int x) { close(x); }
+#define socketerror   errno


Also this has not been fixed.

ghewgill · 2024-02-06T22:07:37Z

lib/net.cpp

@@ -15,6 +15,25 @@

 // https://wiki.openssl.org/index.php/SSL/TLS_Client

+static std::string socket_error(int err)


You seem to have something named socketerror and something else named socket_error. Please find names that are more different.

I changed this to error_to_string(), and the macro to GetSocketError().

ghewgill · 2024-02-13T20:39:58Z

lib/net.cpp

+static std::string error_to_string(int err)
+{
+#ifdef _WIN32
+    static char msgbuf[256] = { '\0' };


Why is this declared static? It doesn't need to be static.

Removed static.

ghewgill · 2024-02-13T20:41:11Z

src/socketx.h

@@ -15,5 +16,6 @@ typedef int socklen_t;
 typedef int SOCKET;
 const int INVALID_SOCKET = -1;
 inline void closesocket(int x) { close(x); }
+#define GetSocketError()   errno


This naming convention is inconsistent. Nothing else in the socket library uses CamelCase. Choose something consistent.

Changed name to socket_error()

ghewgill · 2024-02-13T20:42:10Z

lib/net.cpp

@@ -15,6 +15,25 @@

 // https://wiki.openssl.org/index.php/SSL/TLS_Client

+static std::string error_to_string(int err)


This name is too generic - what kind of error? This is specifically related to sockets, so socket should be mentioned in the name somewhere.

Changed name to socket_error_message()

ghewgill requested changes Jan 23, 2024

View reviewed changes

gitlarryf force-pushed the win32-socket-code branch from e8ffd2e to aa8e389 Compare January 30, 2024 19:17

ghewgill requested changes Feb 2, 2024

View reviewed changes

gitlarryf force-pushed the win32-socket-code branch 2 times, most recently from 5fa1f81 to d03a469 Compare February 3, 2024 05:22

ghewgill requested changes Feb 6, 2024

View reviewed changes

gitlarryf force-pushed the win32-socket-code branch from d03a469 to 62a2eb9 Compare February 9, 2024 06:21

ghewgill requested changes Feb 13, 2024

View reviewed changes

Return actual socket errors on Windows.

51e45db

gitlarryf force-pushed the win32-socket-code branch from 62a2eb9 to 51e45db Compare February 13, 2024 21:14

ghewgill merged commit a041415 into ghewgill:main Feb 13, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set Neon to return meaningful socket errors on Windows during socket.recv() calls. #445

Set Neon to return meaningful socket errors on Windows during socket.recv() calls. #445

gitlarryf commented Jan 22, 2024

ghewgill Jan 23, 2024

gitlarryf Jan 30, 2024

ghewgill Feb 2, 2024

ghewgill Jan 23, 2024

gitlarryf Jan 30, 2024

ghewgill Feb 2, 2024

ghewgill Jan 23, 2024

gitlarryf Jan 30, 2024

ghewgill Jan 23, 2024

gitlarryf Jan 30, 2024

ghewgill Jan 23, 2024

gitlarryf Jan 30, 2024

ghewgill Feb 2, 2024

ghewgill Feb 2, 2024

ghewgill Feb 6, 2024

gitlarryf Feb 9, 2024 •

edited

Loading

ghewgill Feb 13, 2024

gitlarryf Feb 13, 2024

ghewgill Feb 13, 2024

gitlarryf Feb 13, 2024

ghewgill Feb 13, 2024

gitlarryf Feb 13, 2024

		@@ -15,6 +15,25 @@

		// https://wiki.openssl.org/index.php/SSL/TLS_Client

		static std::string socket_error(int err)

		@@ -15,6 +15,25 @@

		// https://wiki.openssl.org/index.php/SSL/TLS_Client

		static std::string error_to_string(int err)

Set Neon to return meaningful socket errors on Windows during socket.recv() calls. #445

Set Neon to return meaningful socket errors on Windows during socket.recv() calls. #445

Conversation

gitlarryf commented Jan 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitlarryf Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitlarryf Feb 9, 2024 •

edited

Loading