All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff Hostetler <jeffhost@microsoft.com>,
	Jeff Hostetler <jeffhost@microsoft.com>
Subject: [PATCH v2] simple-ipc: correct ifdefs when NO_PTHREADS is defined
Date: Thu, 20 May 2021 14:22:27 +0000	[thread overview]
Message-ID: <pull.955.v2.git.1621520547726.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.955.git.1621352192238.gitgitgadget@gmail.com>

From: Jeff Hostetler <jeffhost@microsoft.com>

Simple IPC always requires threads (in addition to various
platform-specific IPC support).  Fix the ifdefs in the Makefile
to define SUPPORTS_SIMPLE_IPC when appropriate.

Previously, the Unix version of the code would only verify that
Unix domain sockets were available.

This problem was reported here:
https://lore.kernel.org/git/YKN5lXs4AoK%2FJFTO@coredump.intra.peff.net/T/#m08be8f1942ea8a2c36cfee0e51cdf06489fdeafc

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
    simple-ipc: correct ifdefs when NO_PTHREADS is defined
    
    Here is V2 of this fixup. I've removed the whole file ifdefs and
    replaced them with a simple #ifndef/#error/#endif as a warning.
    
    Jeff

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-955%2Fjeffhostetler%2Ffixup-simple-ipc-no-pthreads-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-955/jeffhostetler/fixup-simple-ipc-no-pthreads-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/955

Range-diff vs v1:

 1:  4adcf35ea6e4 ! 1:  119412f52ff5 simple-ipc: correct ifdefs when NO_PTHREADS is defined
     @@ Makefile: ifdef NO_UNIX_SOCKETS
      -	LIB_OBJS += compat/simple-ipc/ipc-unix-socket.o
       endif
       
     -+# 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.)
     ++# Simple IPC requires threads and platform-specific IPC support.
     ++# Only platforms that have both should include these source files
     ++# in the build.
     ++#
     ++# On Windows-based systems, Simple IPC requires threads and Windows
     ++# Named Pipes.  These are always available, so Simple IPC support
     ++# is optional.
     ++#
     ++# On Unix-based systems, Simple IPC requires pthreads and Unix
     ++# domain sockets.  So support is only enabled when both are present.
     ++#
       ifdef USE_WIN32_IPC
      +	BASIC_CFLAGS += -DSUPPORTS_SIMPLE_IPC
       	LIB_OBJS += compat/simple-ipc/ipc-shared.o
     @@ Makefile: ifdef NO_UNIX_SOCKETS
       
       ifdef NO_ICONV
      
     + ## compat/simple-ipc/ipc-shared.c ##
     +@@
     + #include "pkt-line.h"
     + #include "thread-utils.h"
     + 
     +-#ifdef SUPPORTS_SIMPLE_IPC
     ++#ifndef SUPPORTS_SIMPLE_IPC
     ++/*
     ++ * This source file should only be compiled when Simple IPC is supported.
     ++ * See the top-level Makefile.
     ++ */
     ++#error SUPPORTS_SIMPLE_IPC not defined
     ++#endif
     + 
     + int ipc_server_run(const char *path, const struct ipc_server_opts *opts,
     + 		   ipc_server_application_cb *application_cb,
     +@@ compat/simple-ipc/ipc-shared.c: int ipc_server_run(const char *path, const struct ipc_server_opts *opts,
     + 
     + 	return ret;
     + }
     +-
     +-#endif /* SUPPORTS_SIMPLE_IPC */
     +
       ## compat/simple-ipc/ipc-unix-socket.c ##
      @@
       #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
     +-#ifdef NO_UNIX_SOCKETS
     +-#error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets
     ++#ifndef SUPPORTS_SIMPLE_IPC
     ++/*
     ++ * This source file should only be compiled when Simple IPC is supported.
     ++ * See the top-level Makefile.
     ++ */
     ++#error SUPPORTS_SIMPLE_IPC not defined
       #endif
       
     -+#ifdef NO_PTHREADS
     -+#error compat/simple-ipc/ipc-unix-socket.c requires pthreads
     -+#endif
     -+
       enum ipc_active_state ipc_get_active_state(const char *path)
     - {
     - 	enum ipc_active_state state = IPC_STATE__OTHER_ERROR;
     -@@ compat/simple-ipc/ipc-unix-socket.c: void ipc_server_free(struct ipc_server_data *server_data)
     - 	free(server_data->fifo_fds);
     - 	free(server_data);
     - }
     -+
     -+#endif /* SUPPORTS_SIMPLE_IPC */
      
       ## compat/simple-ipc/ipc-win32.c ##
      @@
       #include "pkt-line.h"
       #include "thread-utils.h"
       
     -+#ifdef SUPPORTS_SIMPLE_IPC
     -+
     - #ifndef GIT_WINDOWS_NATIVE
     - #error This file can only be compiled on Windows
     +-#ifndef GIT_WINDOWS_NATIVE
     +-#error This file can only be compiled on Windows
     ++#ifndef SUPPORTS_SIMPLE_IPC
     ++/*
     ++ * This source file should only be compiled when Simple IPC is supported.
     ++ * See the top-level Makefile.
     ++ */
     ++#error SUPPORTS_SIMPLE_IPC not defined
       #endif
     -@@ compat/simple-ipc/ipc-win32.c: void ipc_server_free(struct ipc_server_data *server_data)
       
     - 	free(server_data);
     - }
     -+
     -+#endif /* SUPPORTS_SIMPLE_IPC */
     + static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)
      
       ## simple-ipc.h ##
      @@


 Makefile                            | 22 ++++++++++++++++++++--
 compat/simple-ipc/ipc-shared.c      | 10 +++++++---
 compat/simple-ipc/ipc-unix-socket.c |  8 ++++++--
 compat/simple-ipc/ipc-win32.c       |  8 ++++++--
 simple-ipc.h                        |  4 ----
 5 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 3a2d3c80a81a..ea4c0a77604d 100644
--- a/Makefile
+++ b/Makefile
@@ -1687,13 +1687,31 @@ ifdef NO_UNIX_SOCKETS
 else
 	LIB_OBJS += unix-socket.o
 	LIB_OBJS += unix-stream-server.o
-	LIB_OBJS += compat/simple-ipc/ipc-shared.o
-	LIB_OBJS += compat/simple-ipc/ipc-unix-socket.o
 endif
 
+# Simple IPC requires threads and platform-specific IPC support.
+# Only platforms that have both should include these source files
+# in the build.
+#
+# On Windows-based systems, Simple IPC requires threads and Windows
+# Named Pipes.  These are always available, so Simple IPC support
+# is optional.
+#
+# On Unix-based systems, Simple IPC requires pthreads and Unix
+# domain sockets.  So support is only enabled when both are present.
+#
 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
 endif
 
 ifdef NO_ICONV
diff --git a/compat/simple-ipc/ipc-shared.c b/compat/simple-ipc/ipc-shared.c
index 1edec8159532..1b9d359ab681 100644
--- a/compat/simple-ipc/ipc-shared.c
+++ b/compat/simple-ipc/ipc-shared.c
@@ -4,7 +4,13 @@
 #include "pkt-line.h"
 #include "thread-utils.h"
 
-#ifdef SUPPORTS_SIMPLE_IPC
+#ifndef SUPPORTS_SIMPLE_IPC
+/*
+ * This source file should only be compiled when Simple IPC is supported.
+ * See the top-level Makefile.
+ */
+#error SUPPORTS_SIMPLE_IPC not defined
+#endif
 
 int ipc_server_run(const char *path, const struct ipc_server_opts *opts,
 		   ipc_server_application_cb *application_cb,
@@ -24,5 +30,3 @@ int ipc_server_run(const char *path, const struct ipc_server_opts *opts,
 
 	return ret;
 }
-
-#endif /* SUPPORTS_SIMPLE_IPC */
diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c
index 38689b278df3..1927e6ef4bca 100644
--- a/compat/simple-ipc/ipc-unix-socket.c
+++ b/compat/simple-ipc/ipc-unix-socket.c
@@ -6,8 +6,12 @@
 #include "unix-socket.h"
 #include "unix-stream-server.h"
 
-#ifdef NO_UNIX_SOCKETS
-#error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets
+#ifndef SUPPORTS_SIMPLE_IPC
+/*
+ * This source file should only be compiled when Simple IPC is supported.
+ * See the top-level Makefile.
+ */
+#error SUPPORTS_SIMPLE_IPC not defined
 #endif
 
 enum ipc_active_state ipc_get_active_state(const char *path)
diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c
index 8f89c02037e3..8dc7bda087da 100644
--- a/compat/simple-ipc/ipc-win32.c
+++ b/compat/simple-ipc/ipc-win32.c
@@ -4,8 +4,12 @@
 #include "pkt-line.h"
 #include "thread-utils.h"
 
-#ifndef GIT_WINDOWS_NATIVE
-#error This file can only be compiled on Windows
+#ifndef SUPPORTS_SIMPLE_IPC
+/*
+ * This source file should only be compiled when Simple IPC is supported.
+ * See the top-level Makefile.
+ */
+#error SUPPORTS_SIMPLE_IPC not defined
 #endif
 
 static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)
diff --git a/simple-ipc.h b/simple-ipc.h
index dc3606e30bd6..2c48a5ee0047 100644
--- a/simple-ipc.h
+++ b/simple-ipc.h
@@ -5,10 +5,6 @@
  * See Documentation/technical/api-simple-ipc.txt
  */
 
-#if defined(GIT_WINDOWS_NATIVE) || !defined(NO_UNIX_SOCKETS)
-#define SUPPORTS_SIMPLE_IPC
-#endif
-
 #ifdef SUPPORTS_SIMPLE_IPC
 #include "pkt-line.h"
 

base-commit: bf949ade81106fbda068c1fdb2c6fd1cb1babe7e
-- 
gitgitgadget

  parent reply	other threads:[~2021-05-20 14:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-18 15:36 [PATCH] simple-ipc: correct ifdefs when NO_PTHREADS is defined Jeff Hostetler via GitGitGadget
2021-05-19  0:48 ` Junio C Hamano
2021-05-19 14:29   ` Jeff Hostetler
2021-05-19 22:03     ` Junio C Hamano
2021-05-20 14:22 ` Jeff Hostetler via GitGitGadget [this message]
2021-05-20 15:11   ` [PATCH v2] " Jeff Hostetler
2021-05-20 18:28   ` [PATCH v3] " Jeff Hostetler via GitGitGadget
2021-05-20 23:01     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.955.v2.git.1621520547726.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jeffhost@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.