Re: [PATCH] simple-ipc: correct ifdefs when NO_PTHREADS is defined
From: Junio C Hamano <hidden>
Date: 2021-05-19 00:48:30
"Jeff Hostetler via GitGitGadget" [off-list ref] writes:
+# Simple-ipc requires threads and platform-specific IPC support. +# (We group all Unix variants in the top-level else because Windows +# also defines NO_UNIX_SOCKETS.) ifdef USE_WIN32_IPC + BASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC LIB_OBJS += compat/simple-ipc/ipc-shared.o LIB_OBJS += compat/simple-ipc/ipc-win32.o +else +ifndef NO_PTHREADS +ifndef NO_UNIX_SOCKETS + BASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC + LIB_OBJS += compat/simple-ipc/ipc-shared.o + LIB_OBJS += compat/simple-ipc/ipc-unix-socket.o +endif +endif
OK, so "!defined(NO_PTHREADS) && !defined(NO_UNIX_SOCKETS)" is the requirement for SIMPLE_IPC unless you are on Windows.
quoted hunk
diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c index 38689b278df3..c23b17973983 100644 --- a/compat/simple-ipc/ipc-unix-socket.c +++ b/compat/simple-ipc/ipc-unix-socket.c@@ -6,10 +6,16 @@ #include "unix-socket.h" #include "unix-stream-server.h" +#ifdef SUPPORTS_SIMPLE_IPC + #ifdef NO_UNIX_SOCKETS #error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets #endif +#ifdef NO_PTHREADS +#error compat/simple-ipc/ipc-unix-socket.c requires pthreads +#endif +
Do we want to duplicate the requirement here and risk them drifting apart?
quoted hunk
@@ -997,3 +1003,5 @@ void ipc_server_free(struct ipc_server_data *server_data) free(server_data->fifo_fds); free(server_data); } + +#endif /* SUPPORTS_SIMPLE_IPC */
If anything, I do not think we want a huge #ifdef/#endif around the whole file. Feeding this source to your compiler when these three C proprocessor macros do not agree with its use is an error, so perhaps lose all of these #ifdef/#endif around the three macros and refer human readers to the top-level Makefile with a comment, e.g. /* * Non Windows platforms need !NO_UNIX_SOCKETS and !NO_PTHREADS * to compile and use this file. See the top-level Makefile. */ if we really wanted to have a way to help builders identify the reason why their build is failing.