cmake: fix support for UnixSockets feature on Win32 #7034

lixinwei715 · 2021-05-08T08:28:25Z

Move the definition of sockaddr_un struct from config-win32.h to curl_setup.h, so that it could be shared by all build systems.

Add ADDRESS_FAMILY typedef for old mingw, now old mingw can also use unix sockets.

Also fix the build of tests/server/sws.c on Win32 when USE_UNIX_SOCKETS is defined.

Fixes #7029

lixinwei715 · 2021-05-08T12:02:28Z

tests/server/sws.exe build failed, because Windows does not have lstat() function
tests/server/sws.c line 2087~2089:

      /* socket server is not alive, now check if it was actually a socket.
       * Systems which have Unix sockets will also have lstat */
      rc = lstat(unix_socket, &statbuf);

How to fix it?

bagder · 2021-05-08T12:10:56Z

Systems which have Unix sockets will also have lstat

Obviously then a wrong comment! =)

I figure you can just use stat() then, since symbolic links are not used on windows anyway.

bagder · 2021-05-11T06:28:20Z

@vszakats you brought 23a870f, any thoughts on this PR?

vszakats · 2021-05-11T12:14:36Z

The gist of this seems to move the existing UnixSockets trick to curl_addrinfo.c, making it available for UWP, but the CMake-specific logic and moving the small header stub into a separate file look unnecessary. Also the, sws.c changes seem unrelated to this patch, which is okay, but would probably be best in a separate patch to make this easier to understand.

Update: Moving around the UnixSockets trick also seems unnecessary, because UWP apps aren't an issue here (as introduced in 79a05e1).

vszakats · 2021-05-11T12:27:37Z

I didn't test it, but would this simpler patch solve the CMake build issue?:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2ffb72faf..757599767 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -793,7 +793,9 @@ endif()
 option(ENABLE_UNIX_SOCKETS "Define if you want Unix domain sockets support" ON)
 if(ENABLE_UNIX_SOCKETS)
   include(CheckStructHasMember)
-  check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  if(NOT WIN32)
+    check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  endif()
 else()
   unset(USE_UNIX_SOCKETS CACHE)
 endif()

lixinwei715 · 2021-05-11T12:42:31Z

I didn't test it, but would this simpler patch solve the CMake build issue?:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2ffb72faf..757599767 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -793,7 +793,9 @@ endif()
 option(ENABLE_UNIX_SOCKETS "Define if you want Unix domain sockets support" ON)
 if(ENABLE_UNIX_SOCKETS)
   include(CheckStructHasMember)
-  check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  if(NOT WIN32)
+    check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  endif()
 else()
   unset(USE_UNIX_SOCKETS CACHE)
 endif()

Only changing CMakeLists.txt is not enough.
Please view the description of issue #7029.

lixinwei715 · 2021-05-11T13:00:11Z

The gist of this seems to move the existing UnixSockets trick to curl_addrinfo.c, making it available for UWP, but the CMake-specific logic and moving the small header stub into a separate file look unnecessary. Also the, sws.c changes seem unrelated to this patch, which is okay, but would probably be best in a separate patch to make this easier to understand.

Update: Moving around the UnixSockets trick also seems unnecessary, because UWP apps aren't an issue here (as introduced in 79a05e1).

The UnixSockets trick only exists in config-win32.h, but cmake does not use config-win32.h. So I move the UnixSockets trick to a new header file "unixsockets_win32.h". But according to @bagder 's comment in #7029 , we can also move it to curl_config.h. If you think there is no need to create a new header file, I will move the UnixSockets trick to curl_config.h.
Without changing sws.c, compile on appveyor will fail, when USE_UNIX_SOCKETS is ON and Win32 platform. So sws.c changes is related to this patch.

vszakats · 2021-05-11T17:03:08Z

@lixinwei715 Thanks for this info, I understand now. It looks a little bit weird that CMake doesn't use config-win32.h though. It makes things much more bumpy for CMake. I wonder what is the benefit/reason behind it not using it?

[ FWIW I tried moving to CMake in curl-for-win, but it was so limited compared to the bare-bones Makefile.m32 method, that I had to shelve the effort. ]

Re: to sws.c, Makefile.m32 doesn't build tests at all, so that's indeed related when built with CMake, which does.

vszakats · 2021-05-11T18:15:40Z

The best place for the Windows UnixSockets header trick is where it is moved by this patch (curl_setup.h). curl_config.h isn't used by some build systems, such as Makefile.m32, and possibly winbuild.

lixinwei715 · 2021-05-12T03:04:31Z

@vszakats cmake doesn't use config-win32.h, that's because cmake will generate its curl_config.h, based on curl_config.h.cmake.
Compared to other build systems used by curl, there are still some issues on cmake indeed. I plan to commit some PRs to amend the cmake build system in the next few weeks.

lib/curl_setup.h

ghost · 2021-06-02T09:59:10Z

Congratulations 🎉. DeepCode analyzed your code in 2.397 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

vszakats

The new WIN32 header check in CMakeLists.txt will always be true (it is looking for a header inside curl's own source tree), so I'm not convinced that the newly added header (and the check) is necessary here.
The extra UWP logic was stripped from the last version of this PR. I can't test it, but if WIN32 is defined for UWP apps, this should work.

Modified patch below attempts to move Unix Sockets detection (and any necessary tricks) into curl_setup.h, so all Windows builds systems can use it.

@dmitrykos, @lixinwei715 Can you verify/test if this works for CMake and keeps working for UWP apps?

UPDATE: re-added CMake-level pre-detection via curl_setup.h.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1f49dcf3b..b1f4cc86b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -806,7 +806,13 @@ endif()
 option(ENABLE_UNIX_SOCKETS "Define if you want Unix domain sockets support" ON)
 if(ENABLE_UNIX_SOCKETS)
   include(CheckStructHasMember)
-  check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  if(WIN32)
+    set(CMAKE_REQUIRED_INCLUDES "${CMAKE_CURRENT_SOURCE_DIR}/include")
+    check_struct_has_member("struct sockaddr_un" sun_path
+      "${CMAKE_CURRENT_SOURCE_DIR}/lib/curl_setup.h" USE_UNIX_SOCKETS)
+  else()
+    check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  endif()
 else()
   unset(USE_UNIX_SOCKETS CACHE)
 endif()
diff --git a/lib/config-win32.h b/lib/config-win32.h
index fc8153683..5e4997bf0 100644
--- a/lib/config-win32.h
+++ b/lib/config-win32.h
@@ -693,23 +693,6 @@ Vista
 #define USE_WIN32_CRYPTO
 #endif
 
-/* On MinGW the ADDRESS_FAMILY typedef was committed alongside LUP_SECURE,
-   so we use it to check for the presence of the typedef. */
-#include <ws2tcpip.h>
-#if !defined(__MINGW32__) || defined(LUP_SECURE)
-/* Define to use Unix sockets. */
-#define USE_UNIX_SOCKETS
-#if !defined(UNIX_PATH_MAX)
-  /* Replicating logic present in afunix.h of newer Windows 10 SDK versions */
-# define UNIX_PATH_MAX 108
-  /* !checksrc! disable TYPEDEFSTRUCT 1 */
-  typedef struct sockaddr_un {
-    ADDRESS_FAMILY sun_family;
-    char sun_path[UNIX_PATH_MAX];
-  } SOCKADDR_UN, *PSOCKADDR_UN;
-#endif
-#endif
-
 /* ---------------------------------------------------------------- */
 /*                       ADDITIONAL DEFINITIONS                     */
 /* ---------------------------------------------------------------- */
diff --git a/lib/curl_addrinfo.c b/lib/curl_addrinfo.c
index 1d5067bc0..842fd7fe2 100644
--- a/lib/curl_addrinfo.c
+++ b/lib/curl_addrinfo.c
@@ -50,12 +50,6 @@
 #  define in_addr_t unsigned long
 #endif
 
-#if defined(USE_UNIX_SOCKETS) && defined(WINAPI_FAMILY) && \
-    (WINAPI_FAMILY == WINAPI_FAMILY_APP)
-   /* Required for sockaddr_un type */
-#  include <afunix.h>
-#endif
-
 #include <stddef.h>
 
 #include "curl_addrinfo.h"
diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 0b575ef78..76d63dc3d 100644
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -815,4 +815,24 @@ int getpwuid_r(uid_t uid, struct passwd *pwd, char *buf,
 #define ENABLE_QUIC
 #endif
 
+/* Detect if we can use Unix Sockets on Windows,
+   then do any necessary preparations */
+#if defined(WIN32) && !defined(_WIN32_WCE)
+   /* On MinGW the ADDRESS_FAMILY typedef was committed alongside LUP_SECURE,
+      so we use it to check for the presence of the typedef. */
+#  if !defined(__MINGW32__) || defined(LUP_SECURE)
+#    define USE_UNIX_SOCKETS
+#    if !defined(UNIX_PATH_MAX)  /* Check if afunix.h was already included */
+       /* Replicate logic from afunix.h
+          (distributed with newer Windows 10 SDK versions only) */
+#      define UNIX_PATH_MAX 108
+       /* !checksrc! disable TYPEDEFSTRUCT 1 */
+       typedef struct sockaddr_un {
+         ADDRESS_FAMILY sun_family;
+         char sun_path[UNIX_PATH_MAX];
+       } SOCKADDR_UN, *PSOCKADDR_UN;
+#    endif
+#  endif
+#endif
+
 #endif /* HEADER_CURL_SETUP_H */
diff --git a/tests/server/sws.c b/tests/server/sws.c
index b1137414d..e60e9dfbe 100644
--- a/tests/server/sws.c
+++ b/tests/server/sws.c
@@ -2065,9 +2065,9 @@ int main(int argc, char *argv[])
     strncpy(me.sau.sun_path, unix_socket, sizeof(me.sau.sun_path) - 1);
     rc = bind(sock, &me.sa, sizeof(me.sau));
     if(0 != rc && errno == EADDRINUSE) {
-      struct stat statbuf;
+      struct_stat statbuf;
       /* socket already exists. Perhaps it is stale? */
-      int unixfd = socket(AF_UNIX, SOCK_STREAM, 0);
+      curl_socket_t unixfd = socket(AF_UNIX, SOCK_STREAM, 0);
       if(CURL_SOCKET_BAD == unixfd) {
         error = SOCKERRNO;
         logmsg("Error binding socket, failed to create socket at %s: (%d) %s",
@@ -2077,15 +2077,19 @@ int main(int argc, char *argv[])
       /* check whether the server is alive */
       rc = connect(unixfd, &me.sa, sizeof(me.sau));
       error = errno;
-      close(unixfd);
+      sclose(unixfd);
       if(ECONNREFUSED != error) {
         logmsg("Error binding socket, failed to connect to %s: (%d) %s",
                unix_socket, error, strerror(error));
         goto sws_cleanup;
       }
-      /* socket server is not alive, now check if it was actually a socket.
-       * Systems which have Unix sockets will also have lstat */
+      /* socket server is not alive, now check if it was actually a socket. */
+#ifdef WIN32
+      /* Windows does not have lstat function. */
+      rc = curlx_win32_stat(unix_socket, &statbuf);
+#else
       rc = lstat(unix_socket, &statbuf);
+#endif
       if(0 != rc) {
         logmsg("Error binding socket, failed to stat %s: (%d) %s",
                unix_socket, errno, strerror(errno));

lixinwei715 · 2021-06-02T14:09:37Z

CMakeLists.txt

-  check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  if(WIN32)
+    check_struct_has_member("struct sockaddr_un" sun_path
+                            "${CMAKE_CURRENT_SOURCE_DIR}/lib/unixsockets_win32.h" USE_UNIX_SOCKETS)


CMake will print a summary in the end, like this:

-- Enabled features: SSL IPv6 libz brotli zstd AsynchDNS IDN Largefile SSPI alt-svc HSTS SPNEGO Kerberos NTLM HTTP2 unicode -- Enabled protocols: DICT FILE FTP FTPS GOPHER GOPHERS HTTP HTTPS IMAP IMAPS LDAP LDAPS MQTT POP3 POP3S RTSP SCP SFTP SMB SMBS SMTP SMTPS TELNET TFTP -- Enabled SSL backends: Schannel

The check of unixsockets should be done by cmake. So that we can see "unixsockets" in "Enabled features" before compiling.

You move Unix Sockets detection to curl_setup.h, this means whether unix sockets enable of disable will be automatically determined, so that cmake option "ENABLE_UNIX_SOCKETS" doesn't work on Windows. Users cannot disable unix sockets feature by seting ENABLE_UNIX_SOCKETS to OFF.

Could this code work for detection in CMake?:

if(WIN32) set(CMAKE_REQUIRED_INCLUDES "${CMAKE_CURRENT_SOURCE_DIR}/include") check_struct_has_member("struct sockaddr_un" sun_path "${CMAKE_CURRENT_SOURCE_DIR}/lib/curl_setup.h" USE_UNIX_SOCKETS)

lib/curl_setup.h

lixinwei715 · 2021-06-02T14:44:39Z

The new WIN32 header check in CMakeLists.txt will always be true

No, for 10y+ old mingw, !defined(__MINGW32__) || defined(LUP_SECURE) is false, sockaddr_un will not defined, the result of check_struct_has_member will be false.

If you want to know how check_struct_has_member works, please view:
https://github.com/Kitware/CMake/blob/master/Modules/CheckStructHasMember.cmake

vszakats · 2021-06-02T15:47:51Z

@lixinwei715 Got it now, it's a pre-check for CMake to know (not for the file itself, but for the actual struct). Can you take a look at #7034 (comment)?

vszakats · 2021-06-02T17:42:06Z

It looks like we're trying to achieve slightly different things. The title says 'fix UnixSockets support with CMake on Windows'. Fixing this doesn't need duplicate CMake-level feature detection. Other Windows build systems enable this feature automatically at compile-time, so it's expected for CMake to behave the same.

So, I'd personally keep it simple and not add an extra source file to detect one feature on a single platform and single make system, and instead prefer to keep the already-too-messy Windows detection/enabler logic in one block of code shared by all build systems and compilers.

If we want to implement a new feature where Unix Sockets can be manually force-disabled, a CURL_DISABLE_UNIX_SOCKETS or similar setting can be added, and make CMake set it when ENABLE_UNIX_SOCKETS was set to 0. I've added it to this patch:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1f49dcf3b..29cb6f4e3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -805,9 +805,12 @@ endif()
 
 option(ENABLE_UNIX_SOCKETS "Define if you want Unix domain sockets support" ON)
 if(ENABLE_UNIX_SOCKETS)
-  include(CheckStructHasMember)
-  check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  if(NOT WIN32)  # On Windows, Unix sockets detection is done in curl_setup.h
+    include(CheckStructHasMember)
+    check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  endif()
 else()
+  set(CURL_DISABLE_UNIX_SOCKETS ON)
   unset(USE_UNIX_SOCKETS CACHE)
 endif()
 
diff --git a/lib/config-win32.h b/lib/config-win32.h
index fc8153683..5e4997bf0 100644
--- a/lib/config-win32.h
+++ b/lib/config-win32.h
@@ -693,23 +693,6 @@ Vista
 #define USE_WIN32_CRYPTO
 #endif
 
-/* On MinGW the ADDRESS_FAMILY typedef was committed alongside LUP_SECURE,
-   so we use it to check for the presence of the typedef. */
-#include <ws2tcpip.h>
-#if !defined(__MINGW32__) || defined(LUP_SECURE)
-/* Define to use Unix sockets. */
-#define USE_UNIX_SOCKETS
-#if !defined(UNIX_PATH_MAX)
-  /* Replicating logic present in afunix.h of newer Windows 10 SDK versions */
-# define UNIX_PATH_MAX 108
-  /* !checksrc! disable TYPEDEFSTRUCT 1 */
-  typedef struct sockaddr_un {
-    ADDRESS_FAMILY sun_family;
-    char sun_path[UNIX_PATH_MAX];
-  } SOCKADDR_UN, *PSOCKADDR_UN;
-#endif
-#endif
-
 /* ---------------------------------------------------------------- */
 /*                       ADDITIONAL DEFINITIONS                     */
 /* ---------------------------------------------------------------- */
diff --git a/lib/curl_addrinfo.c b/lib/curl_addrinfo.c
index 1d5067bc0..842fd7fe2 100644
--- a/lib/curl_addrinfo.c
+++ b/lib/curl_addrinfo.c
@@ -50,12 +50,6 @@
 #  define in_addr_t unsigned long
 #endif
 
-#if defined(USE_UNIX_SOCKETS) && defined(WINAPI_FAMILY) && \
-    (WINAPI_FAMILY == WINAPI_FAMILY_APP)
-   /* Required for sockaddr_un type */
-#  include <afunix.h>
-#endif
-
 #include <stddef.h>
 
 #include "curl_addrinfo.h"
diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 0b575ef78..bf6aedb45 100644
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -815,4 +815,25 @@ int getpwuid_r(uid_t uid, struct passwd *pwd, char *buf,
 #define ENABLE_QUIC
 #endif
 
+/* Detect if we can use Unix Sockets on Windows,
+   then do any necessary preparations */
+#if defined(WIN32) && !defined(_WIN32_WCE) && \
+    !defined(CURL_DISABLE_UNIX_SOCKETS)
+   /* On MinGW the ADDRESS_FAMILY typedef was committed alongside LUP_SECURE,
+      so we use it to check for the presence of the typedef. */
+#  if !defined(__MINGW32__) || defined(LUP_SECURE)
+#    define USE_UNIX_SOCKETS
+#    if !defined(UNIX_PATH_MAX)  /* Check if afunix.h was already included */
+       /* Replicate logic in afunix.h
+          (distributed with newer Windows 10 SDK versions only) */
+#      define UNIX_PATH_MAX 108
+       /* !checksrc! disable TYPEDEFSTRUCT 1 */
+       typedef struct sockaddr_un {
+         ADDRESS_FAMILY sun_family;
+         char sun_path[UNIX_PATH_MAX];
+       } SOCKADDR_UN, *PSOCKADDR_UN;
+#    endif
+#  endif
+#endif
+
 #endif /* HEADER_CURL_SETUP_H */
diff --git a/tests/server/sws.c b/tests/server/sws.c
index b1137414d..e60e9dfbe 100644
--- a/tests/server/sws.c
+++ b/tests/server/sws.c
@@ -2065,9 +2065,9 @@ int main(int argc, char *argv[])
     strncpy(me.sau.sun_path, unix_socket, sizeof(me.sau.sun_path) - 1);
     rc = bind(sock, &me.sa, sizeof(me.sau));
     if(0 != rc && errno == EADDRINUSE) {
-      struct stat statbuf;
+      struct_stat statbuf;
       /* socket already exists. Perhaps it is stale? */
-      int unixfd = socket(AF_UNIX, SOCK_STREAM, 0);
+      curl_socket_t unixfd = socket(AF_UNIX, SOCK_STREAM, 0);
       if(CURL_SOCKET_BAD == unixfd) {
         error = SOCKERRNO;
         logmsg("Error binding socket, failed to create socket at %s: (%d) %s",
@@ -2077,15 +2077,19 @@ int main(int argc, char *argv[])
       /* check whether the server is alive */
       rc = connect(unixfd, &me.sa, sizeof(me.sau));
       error = errno;
-      close(unixfd);
+      sclose(unixfd);
       if(ECONNREFUSED != error) {
         logmsg("Error binding socket, failed to connect to %s: (%d) %s",
                unix_socket, error, strerror(error));
         goto sws_cleanup;
       }
-      /* socket server is not alive, now check if it was actually a socket.
-       * Systems which have Unix sockets will also have lstat */
+      /* socket server is not alive, now check if it was actually a socket. */
+#ifdef WIN32
+      /* Windows does not have lstat function. */
+      rc = curlx_win32_stat(unix_socket, &statbuf);
+#else
       rc = lstat(unix_socket, &statbuf);
+#endif
       if(0 != rc) {
         logmsg("Error binding socket, failed to stat %s: (%d) %s",
                unix_socket, errno, strerror(errno));

lixinwei715 · 2021-06-02T18:06:23Z

As far as I know, the reason we disable unix sockets for old mingw is that old mingw missing ADDRESS_FAMILY typedef.
Could we add this typedef maually? So that unix sockets could be enabled on all Windows, cmake don't need to check.

#if defined(__MINGW32__) && !defined(LUP_SECURE)
  typedef u_short ADDRESS_FAMILY;
#endif
#if !defined(UNIX_PATH_MAX)
  /* Replicating logic present in afunix.h of newer Windows 10 SDK versions */
# define UNIX_PATH_MAX 108
  /* !checksrc! disable TYPEDEFSTRUCT 1 */
  typedef struct sockaddr_un {
    ADDRESS_FAMILY sun_family;
    char sun_path[UNIX_PATH_MAX];
  } SOCKADDR_UN, *PSOCKADDR_UN;
#endif

lixinwei715 · 2021-06-02T18:44:53Z

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2ffb72faf..6fcb68e79 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -793,7 +793,11 @@ endif()
 option(ENABLE_UNIX_SOCKETS "Define if you want Unix domain sockets support" ON)
 if(ENABLE_UNIX_SOCKETS)
   include(CheckStructHasMember)
-  check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  if(WIN32 AND NOT WINCE)
+    set(USE_UNIX_SOCKETS ON)
+  else()
+    check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  endif()
 else()
   unset(USE_UNIX_SOCKETS CACHE)
 endif()
diff --git a/lib/config-win32.h b/lib/config-win32.h
index 2e060f95c..5fca98c16 100644
--- a/lib/config-win32.h
+++ b/lib/config-win32.h
@@ -713,22 +713,8 @@ Vista
 #define USE_WIN32_CRYPTO
 #endif
 
-/* On MinGW the ADDRESS_FAMILY typedef was committed alongside LUP_SECURE,
-   so we use it to check for the presence of the typedef. */
-#include <ws2tcpip.h>
-#if !defined(__MINGW32__) || defined(LUP_SECURE)
-/* Define to use Unix sockets. */
+/* If Unix domain sockets are enabled. */
 #define USE_UNIX_SOCKETS
-#if !defined(UNIX_PATH_MAX)
-  /* Replicating logic present in afunix.h of newer Windows 10 SDK versions */
-# define UNIX_PATH_MAX 108
-  /* !checksrc! disable TYPEDEFSTRUCT 1 */
-  typedef struct sockaddr_un {
-    ADDRESS_FAMILY sun_family;
-    char sun_path[UNIX_PATH_MAX];
-  } SOCKADDR_UN, *PSOCKADDR_UN;
-#endif
-#endif
 
 /* ---------------------------------------------------------------- */
 /*                       ADDITIONAL DEFINITIONS                     */
diff --git a/lib/curl_addrinfo.c b/lib/curl_addrinfo.c
index 1d5067bc0..842fd7fe2 100644
--- a/lib/curl_addrinfo.c
+++ b/lib/curl_addrinfo.c
@@ -50,12 +50,6 @@
 #  define in_addr_t unsigned long
 #endif
 
-#if defined(USE_UNIX_SOCKETS) && defined(WINAPI_FAMILY) && \
-    (WINAPI_FAMILY == WINAPI_FAMILY_APP)
-   /* Required for sockaddr_un type */
-#  include <afunix.h>
-#endif
-
 #include <stddef.h>
 
 #include "curl_addrinfo.h"
diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 004accdf1..aa975669f 100644
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -809,4 +809,21 @@ int getpwuid_r(uid_t uid, struct passwd *pwd, char *buf,
 #define ENABLE_QUIC
 #endif
 
+#if defined(USE_UNIX_SOCKETS) && defined(WIN32) && !defined(_WIN32_WCE)
+   /* On MinGW the ADDRESS_FAMILY typedef was committed alongside LUP_SECURE,
+      so we use it to check for the presence of the typedef. */
+#  if defined(__MINGW32__) && !defined(LUP_SECURE)
+     typedef u_short ADDRESS_FAMILY;
+#  endif
+#  if !defined(UNIX_PATH_MAX)
+     /* Replicating logic present in afunix.h of newer Windows 10 SDK versions */
+#    define UNIX_PATH_MAX 108
+     /* !checksrc! disable TYPEDEFSTRUCT 1 */
+     typedef struct sockaddr_un {
+       ADDRESS_FAMILY sun_family;
+       char sun_path[UNIX_PATH_MAX];
+     } SOCKADDR_UN, *PSOCKADDR_UN;
+#  endif
+#endif
+
 #endif /* HEADER_CURL_SETUP_H */
diff --git a/tests/server/sws.c b/tests/server/sws.c
index 99efa3786..b5b179386 100644
--- a/tests/server/sws.c
+++ b/tests/server/sws.c
@@ -2066,9 +2066,9 @@ int main(int argc, char *argv[])
     strncpy(me.sau.sun_path, unix_socket, sizeof(me.sau.sun_path) - 1);
     rc = bind(sock, &me.sa, sizeof(me.sau));
     if(0 != rc && errno == EADDRINUSE) {
-      struct stat statbuf;
+      struct_stat statbuf;
       /* socket already exists. Perhaps it is stale? */
-      int unixfd = socket(AF_UNIX, SOCK_STREAM, 0);
+      curl_socket_t unixfd = socket(AF_UNIX, SOCK_STREAM, 0);
       if(CURL_SOCKET_BAD == unixfd) {
         error = SOCKERRNO;
         logmsg("Error binding socket, failed to create socket at %s: (%d) %s",
@@ -2078,15 +2078,19 @@ int main(int argc, char *argv[])
       /* check whether the server is alive */
       rc = connect(unixfd, &me.sa, sizeof(me.sau));
       error = errno;
-      close(unixfd);
+      sclose(unixfd);
       if(ECONNREFUSED != error) {
         logmsg("Error binding socket, failed to connect to %s: (%d) %s",
                unix_socket, error, strerror(error));
         goto sws_cleanup;
       }
-      /* socket server is not alive, now check if it was actually a socket.
-       * Systems which have Unix sockets will also have lstat */
+      /* socket server is not alive, now check if it was actually a socket. */
+#ifdef WIN32
+      /* Windows does not have lstat function. */
+      rc = curlx_win32_stat(unix_socket, &statbuf);
+#else
       rc = lstat(unix_socket, &statbuf);
+#endif
       if(0 != rc) {
         logmsg("Error binding socket, failed to stat %s: (%d) %s",
                unix_socket, errno, strerror(errno));

vszakats · 2021-06-02T19:11:36Z

@lixinwei715 Yes, this could work! As far as I could verify with a few old mingw releases, u_short is available.

If you agree with this revision, feel free to update this PR with it. Then we can see what the CIs say:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1f49dcf3b..efad9df68 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -805,9 +805,12 @@ endif()
 
 option(ENABLE_UNIX_SOCKETS "Define if you want Unix domain sockets support" ON)
 if(ENABLE_UNIX_SOCKETS)
-  include(CheckStructHasMember)
-  check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  if(NOT WIN32)
+    include(CheckStructHasMember)
+    check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  endif()
 else()
+  set(CURL_DISABLE_UNIX_SOCKETS ON)
   unset(USE_UNIX_SOCKETS CACHE)
 endif()
 
diff --git a/lib/config-win32.h b/lib/config-win32.h
index fc8153683..5e4997bf0 100644
--- a/lib/config-win32.h
+++ b/lib/config-win32.h
@@ -693,23 +693,6 @@ Vista
 #define USE_WIN32_CRYPTO
 #endif
 
-/* On MinGW the ADDRESS_FAMILY typedef was committed alongside LUP_SECURE,
-   so we use it to check for the presence of the typedef. */
-#include <ws2tcpip.h>
-#if !defined(__MINGW32__) || defined(LUP_SECURE)
-/* Define to use Unix sockets. */
-#define USE_UNIX_SOCKETS
-#if !defined(UNIX_PATH_MAX)
-  /* Replicating logic present in afunix.h of newer Windows 10 SDK versions */
-# define UNIX_PATH_MAX 108
-  /* !checksrc! disable TYPEDEFSTRUCT 1 */
-  typedef struct sockaddr_un {
-    ADDRESS_FAMILY sun_family;
-    char sun_path[UNIX_PATH_MAX];
-  } SOCKADDR_UN, *PSOCKADDR_UN;
-#endif
-#endif
-
 /* ---------------------------------------------------------------- */
 /*                       ADDITIONAL DEFINITIONS                     */
 /* ---------------------------------------------------------------- */
diff --git a/lib/curl_addrinfo.c b/lib/curl_addrinfo.c
index 1d5067bc0..842fd7fe2 100644
--- a/lib/curl_addrinfo.c
+++ b/lib/curl_addrinfo.c
@@ -50,12 +50,6 @@
 #  define in_addr_t unsigned long
 #endif
 
-#if defined(USE_UNIX_SOCKETS) && defined(WINAPI_FAMILY) && \
-    (WINAPI_FAMILY == WINAPI_FAMILY_APP)
-   /* Required for sockaddr_un type */
-#  include <afunix.h>
-#endif
-
 #include <stddef.h>
 
 #include "curl_addrinfo.h"
diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 0b575ef78..4dc63cfa8 100644
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -815,4 +815,24 @@ int getpwuid_r(uid_t uid, struct passwd *pwd, char *buf,
 #define ENABLE_QUIC
 #endif
 
+/* Detect if we can use Unix Sockets on Windows,
+   then do any necessary preparations */
+#if defined(WIN32) && !defined(_WIN32_WCE) && \
+    !defined(CURL_DISABLE_UNIX_SOCKETS)
+#  define USE_UNIX_SOCKETS
+#  if defined(__MINGW32__) && !defined(LUP_SECURE)
+     typedef u_short ADDRESS_FAMILY;  /* Classic mingw, 11y+ old mingw-w64 */
+#  endif
+#  if !defined(UNIX_PATH_MAX)  /* Check if afunix.h was already included */
+     /* Replicate logic in afunix.h
+        (distributed with newer Windows 10 SDK versions only) */
+#    define UNIX_PATH_MAX 108
+     /* !checksrc! disable TYPEDEFSTRUCT 1 */
+     typedef struct sockaddr_un {
+       ADDRESS_FAMILY sun_family;
+       char sun_path[UNIX_PATH_MAX];
+     } SOCKADDR_UN, *PSOCKADDR_UN;
+#  endif
+#endif
+
 #endif /* HEADER_CURL_SETUP_H */
diff --git a/tests/server/sws.c b/tests/server/sws.c
index b1137414d..e60e9dfbe 100644
--- a/tests/server/sws.c
+++ b/tests/server/sws.c
@@ -2065,9 +2065,9 @@ int main(int argc, char *argv[])
     strncpy(me.sau.sun_path, unix_socket, sizeof(me.sau.sun_path) - 1);
     rc = bind(sock, &me.sa, sizeof(me.sau));
     if(0 != rc && errno == EADDRINUSE) {
-      struct stat statbuf;
+      struct_stat statbuf;
       /* socket already exists. Perhaps it is stale? */
-      int unixfd = socket(AF_UNIX, SOCK_STREAM, 0);
+      curl_socket_t unixfd = socket(AF_UNIX, SOCK_STREAM, 0);
       if(CURL_SOCKET_BAD == unixfd) {
         error = SOCKERRNO;
         logmsg("Error binding socket, failed to create socket at %s: (%d) %s",
@@ -2077,15 +2077,19 @@ int main(int argc, char *argv[])
       /* check whether the server is alive */
       rc = connect(unixfd, &me.sa, sizeof(me.sau));
       error = errno;
-      close(unixfd);
+      sclose(unixfd);
       if(ECONNREFUSED != error) {
         logmsg("Error binding socket, failed to connect to %s: (%d) %s",
                unix_socket, error, strerror(error));
         goto sws_cleanup;
       }
-      /* socket server is not alive, now check if it was actually a socket.
-       * Systems which have Unix sockets will also have lstat */
+      /* socket server is not alive, now check if it was actually a socket. */
+#ifdef WIN32
+      /* Windows does not have lstat function. */
+      rc = curlx_win32_stat(unix_socket, &statbuf);
+#else
       rc = lstat(unix_socket, &statbuf);
+#endif
       if(0 != rc) {
         logmsg("Error binding socket, failed to stat %s: (%d) %s",
                unix_socket, errno, strerror(errno));

vszakats · 2021-06-02T19:24:27Z

Re: your revision. For clarity, if we enable this by default, let's not require USE_UNIX_SOCKETS from the build systems, and instead require something for disabling it.

dmitrykos · 2021-06-02T19:42:31Z

@vszakats I confirm that it compiles ok for UWP platform.

lixinwei715 · 2021-06-02T19:44:48Z

To be consistent with other platforms' build system, USE_UNIX_SOCKETS macro should still be handled by the build system.

And as far as I know, on build systems that use config-win32.h, USE_UNIX_SOCKETS is always enabled by default. No switch to disable it.

vszakats · 2021-06-02T20:24:51Z

You're right that there was no way to disable it in config-win32.h. But, if we allow it to be disabled for CMake, for consistency within the Windows platform, it'd be best to allow the same for all Windows build systems. Hence my proposal for having it enabled without an extra step (USE_UNIX_SOCKETS) by default and allow CURL_DISABLE_UNIX_SOCKETS to disable it, in all of the Windows build systems.

CURL_DISABLE_UNIX_SOCKETS can be easily extended to all non-Windows platforms, which would make sense given that UnixSockets is enabled by default on every other platform too, when supported. (But most likely this feature is unnecessary for these platforms in practice.)

lixinwei715 · 2021-06-03T07:44:11Z

If using cmake build system, users could set ENABLE_UNIX_SOCKETS to OFF ( by default it is ON ) to disable unix sockets.
If using autoconf build system, users could use --disable-unix-sockets option to disable unix sockets.
On Windows, only build systems that use config-win32.h (Makefile.vc and Makefile.m32) need CURL_DISABLE_UNIX_SOCKETS, so I add CURL_DISABLE_UNIX_SOCKETS to config-win32.h.

However, test 1165 failed:

test 1165...[Verify configure.ac and source code CURL_DISABLE_-sync]

perl -I.  returned 2, when expecting 0
 exit FAILED
== Contents of files in the log/ dir after test 1165
=== Start of file commands.log
 perl -I. ./disable-scan.pl ./.. >log/stdout1165 2>log/stderr1165
=== End of file commands.log
=== Start of file ftpserver.cmd
 Testnum 1165
=== End of file ftpserver.cmd
=== Start of file stdout1165
 Not set by configure: CURL_DISABLE_UNIX_SOCKETS (./../lib/config-win32.h)
 Used in code, not documented in CURL-DISABLE.md: CURL_DISABLE_UNIX_SOCKETS
=== End of file stdout1165

Update: this failure means I should rename CURL_DISABLE_UNIX_SOCKETS to other names, like CURL_WIN32_DISABLE_UNIX_SOCKETS.
Or I shouldn't simply add CURL_DISABLE_UNIX_SOCKETS, I need to provide options to enable or diable unix scokets for Makefile.vc and Makefile.m32 instead.

Update: After renaming to CURL_WIN32_DISABLE_UNIX_SOCKETS, test 1165 now passes. Providing new options for Makefile.vc and Makefile.m32 means a new feature, should be in a new PR. For this PR, I think adding CURL_WIN32_DISABLE_UNIX_SOCKETS temporarily is enough.

vszakats

My vote against CURL_WIN32_DISABLE_UNIX_SOCKETS (it's Windows-specific, but even the WIN32-free name needs many more updates than I thought. Thanks for your test on this.), and I'd vote for moving defining USE_UNIX_SOCKETS to respective Makefile.m32 and Makefile.vc. (Then there are the many Windows builds inside projects/Windows which will likely need to be updated, too.)

lixinwei715 · 2021-06-03T19:01:56Z

Sorry, I didn't realize projects/Windows also need to be updated.
Looks like whether adding CURL_DISABLE_UNIX_SOCKETS for all platforms, or making all windows build systems handle USE_UNIX_SOCKETS, both need a lot of work. I don't know if it is worth to do so.
However, the aim of this PR is to solve the cmake problem, not to solve the problem that no way to disable unix sockets in config-win32.h, the latter should be solved by a new PR.
So I think in this PR, we should still define USE_UNIX_SOCKETS in config-win32.h unconditionally.

Move the definition of sockaddr_un struct from config-win32.h to curl_setup.h, so that it could be shared by all build systems. Add ADDRESS_FAMILY typedef for old mingw, now old mingw can also use unix sockets. Also fix the build of tests/server/sws.c on Win32 when USE_UNIX_SOCKETS is defined.

jay · 2021-06-13T06:49:31Z

CMakeLists.txt

@@ -793,7 +793,11 @@ endif()
 option(ENABLE_UNIX_SOCKETS "Define if you want Unix domain sockets support" ON)
 if(ENABLE_UNIX_SOCKETS)
  include(CheckStructHasMember)
-  check_struct_has_member("struct sockaddr_un" sun_path "sys/un.h" USE_UNIX_SOCKETS)
+  if(WIN32)
+    set(USE_UNIX_SOCKETS ON)


This assumes unix sockets is always available on windows?

Yes.
With this PR, whether msvc or mingw, whether newer verison or older version, compilation will always pass when unix sockets feature is ON.

Ok but do you know unix sockets to always be available on Windows? AFAIK it is only in the most recent builds of Windows.

/cc @vszakats

I think for unix sockets, curl only requires the definition of sockaddr_un structure.
Although unix sockets support on Windows is introduced by Windows 10 SDK 17063. But actually, if we add the definition of sockaddr_un structure manually, it can be extended to all build toolchains on Windows.

See also: 23a870f

A Windows binary that supports UnixSockets can be built regardless of the toolchain or SDK version. As to whether UnixSockets works or not will/may depend on the actual Windows environment the binary is executed in. This patch brings nothing new here, only extends extisting support to CMake.

lixinwei715 · 2021-06-20T14:12:05Z

Could this PR be able to merge?

bagder · 2021-06-21T12:52:50Z

Thanks!

bagder added cmake Windows Windows-specific labels May 8, 2021

lixinwei715 commented May 12, 2021

View reviewed changes

lib/curl_setup.h Outdated Show resolved Hide resolved

bagder requested a review from vszakats June 2, 2021 06:52

vszakats requested changes Jun 2, 2021

View reviewed changes

lixinwei715 requested a review from vszakats June 3, 2021 12:12

vszakats requested changes Jun 3, 2021

View reviewed changes

vszakats approved these changes Jun 8, 2021

View reviewed changes

jay reviewed Jun 13, 2021

View reviewed changes

bagder closed this in 30e491e Jun 21, 2021

mzr mentioned this pull request Sep 19, 2021

remove requirement on HAVE_SYS_UN_H for unix domain sockets on Windows #7737

Closed

cmake: fix support for UnixSockets feature on Win32 #7034

cmake: fix support for UnixSockets feature on Win32 #7034

Conversation

lixinwei715 commented May 8, 2021 • edited Loading

lixinwei715 commented May 8, 2021

bagder commented May 8, 2021

bagder commented May 11, 2021

vszakats commented May 11, 2021 • edited Loading

vszakats commented May 11, 2021 • edited Loading

lixinwei715 commented May 11, 2021

lixinwei715 commented May 11, 2021 • edited Loading

vszakats commented May 11, 2021 • edited Loading

vszakats commented May 11, 2021 • edited Loading

lixinwei715 commented May 12, 2021

ghost commented Jun 2, 2021 • edited by ghost Loading

👉 View analysis in DeepCode’s Dashboard | Configure the bot

vszakats left a comment • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

lixinwei715 Jun 2, 2021

Choose a reason for hiding this comment

lixinwei715 Jun 2, 2021

Choose a reason for hiding this comment

vszakats Jun 2, 2021 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

lixinwei715 commented Jun 2, 2021

vszakats commented Jun 2, 2021 • edited Loading

vszakats commented Jun 2, 2021 • edited Loading

lixinwei715 commented Jun 2, 2021

lixinwei715 commented Jun 2, 2021

vszakats commented Jun 2, 2021

vszakats commented Jun 2, 2021

dmitrykos commented Jun 2, 2021

lixinwei715 commented Jun 2, 2021 • edited Loading

vszakats commented Jun 2, 2021 • edited Loading

lixinwei715 commented Jun 3, 2021 • edited Loading

vszakats left a comment • edited Loading

Choose a reason for hiding this comment

lixinwei715 commented Jun 3, 2021 • edited Loading

jay Jun 13, 2021

Choose a reason for hiding this comment

lixinwei715 Jun 13, 2021

Choose a reason for hiding this comment

jay Jun 13, 2021

Choose a reason for hiding this comment

lixinwei715 Jun 13, 2021 • edited Loading

Choose a reason for hiding this comment

vszakats Jun 14, 2021

Choose a reason for hiding this comment

lixinwei715 commented Jun 20, 2021

bagder commented Jun 21, 2021

lixinwei715 commented May 8, 2021 •

edited

Loading

vszakats commented May 11, 2021 •

edited

Loading

vszakats commented May 11, 2021 •

edited

Loading

lixinwei715 commented May 11, 2021 •

edited

Loading

vszakats commented May 11, 2021 •

edited

Loading

vszakats commented May 11, 2021 •

edited

Loading

ghost commented Jun 2, 2021 •

edited by ghost

Loading

vszakats left a comment •

edited

Loading

vszakats Jun 2, 2021 •

edited

Loading

vszakats commented Jun 2, 2021 •

edited

Loading

vszakats commented Jun 2, 2021 •

edited

Loading

lixinwei715 commented Jun 2, 2021 •

edited

Loading

vszakats commented Jun 2, 2021 •

edited

Loading

lixinwei715 commented Jun 3, 2021 •

edited

Loading

vszakats left a comment •

edited

Loading

lixinwei715 commented Jun 3, 2021 •

edited

Loading

lixinwei715 Jun 13, 2021 •

edited

Loading