[pacman-dev] NO_MSGSIGNAL and SIGPOLL don't exist on darwin

Message ID af36afb2-0dba-eb2e-6b71-51632f4c6d29@gmail.com
State Rejected, archived
Headers show
Series [pacman-dev] NO_MSGSIGNAL and SIGPOLL don't exist on darwin | expand

Commit Message

Cameron Katri Sept. 1, 2020, 2:52 a.m. UTC
On darwin NO_MSGSIGNAL and SIGPOLL, don't exist, so SO_NOSIGPIPE will be used instead, which serves the same function.

From a61c9634831e5aff6cd020148516337dd4f8090f Mon Sep 17 00:00:00 2001
From: Cameron Katri <katri.cameron@gmail.com>
Date: Mon, 31 Aug 2020 09:50:09 -0400
Subject: [PATCH] Fix compilation on Darwin

---
 lib/libalpm/util.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Eli Schwartz Sept. 1, 2020, 3:08 a.m. UTC | #1
On 8/31/20 10:52 PM, Cameron Katri wrote:
> On darwin NO_MSGSIGNAL and SIGPOLL, don't exist, so SO_NOSIGPIPE will be used instead, which serves the same function.

IIUC this isn't usable in send(), but only in setsockopt(), so I think
you'll need a more comprehensive solution to ensure it does more than
compile.

Also why are you hardcoding the value rather than using the existing
SO_NOSIGPIPE define wherever it is found?

> From a61c9634831e5aff6cd020148516337dd4f8090f Mon Sep 17 00:00:00 2001
> From: Cameron Katri <katri.cameron@gmail.com>
> Date: Mon, 31 Aug 2020 09:50:09 -0400
> Subject: [PATCH] Fix compilation on Darwin
> 
> ---
>  lib/libalpm/util.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index b70a8192..f7d40b78 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -34,6 +34,11 @@
>  #include <fnmatch.h>
>  #include <poll.h>
>  
> +/* no MSG_NOSIGNAL support */
> +#ifndef MSG_NOSIGNAL
> +#define MSG_NOSIGNAL 0x1022
> +#endif
> +
>  /* libarchive */
>  #include <archive.h>
>  #include <archive_entry.h>
> @@ -557,8 +562,11 @@ static void _alpm_reset_signals(void)
>  	int *i, signals[] = {
>  		SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, SIGILL,
>  		SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, SIGTSTP,
> -		SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, SIGTRAP,
> +		SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPROF, SIGSYS, SIGTRAP,
>  		SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
> +#ifndef __APPLE__
> +		SIGPOLL,
> +#endif
>  		0
>  	};
>  	struct sigaction def;
>
Cameron Katri Sept. 1, 2020, 7:50 p.m. UTC | #2
I believe I found a more portable solution. We use signal(SIGPIPE, SIG_IGN); which will block SIGPIPE, and if MSG_NOSIGNAL is not supported we set it to 0x0 so that it just gets ignored. Hopefully this is a more comprehensive solution.  

From ea38d52b8301eacaf0b7cd76ba5e1c2bf601f4f5 Mon Sep 17 00:00:00 2001
From: Cameron Katri <katri.cameron@gmail.com>
Date: Mon, 31 Aug 2020 09:50:09 -0400
Subject: [PATCH] Fix compilation on Darwin

---
 lib/libalpm/util.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index b70a8192..4a51b63f 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -34,6 +34,11 @@
 #include <fnmatch.h>
 #include <poll.h>
 
+/* no MSG_NOSIGNAL support */
+#ifndef MSG_NOSIGNAL
+#define MSG_NOSIGNAL 0x0
+#endif
+
 /* libarchive */
 #include <archive.h>
 #include <archive_entry.h>
@@ -474,6 +479,8 @@ static int _alpm_chroot_write_to_child(alpm_handle_t *handle, int fd,
 		}
 	}
 
+	signal(SIGPIPE, SIG_IGN);
+
 	nwrite = send(fd, buf, *buf_size, MSG_NOSIGNAL);
 
 	if(nwrite != -1) {
@@ -557,8 +564,11 @@ static void _alpm_reset_signals(void)
 	int *i, signals[] = {
 		SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, SIGILL,
 		SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, SIGTSTP,
-		SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, SIGTRAP,
+		SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPROF, SIGSYS, SIGTRAP,
 		SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
+#ifndef __APPLE__
+		SIGPOLL,
+#endif
 		0
 	};
 	struct sigaction def;
Andrew Gregory Sept. 4, 2020, 1:25 a.m. UTC | #3
On 09/01/20 at 03:50pm, Cameron Katri wrote:
> I believe I found a more portable solution. We use signal(SIGPIPE,
> SIG_IGN); which will block SIGPIPE, and if MSG_NOSIGNAL is not
> supported we set it to 0x0 so that it just gets ignored. Hopefully
> this is a more comprehensive solution.  
...
>  
> +	signal(SIGPIPE, SIG_IGN);

NACK. Signal handlers are process-wide and not something libraries
should be modifying without very good reason, especially not
conditionally and mid-operation.  The fact that gpgme ignores SIGPIPE
is what broke scripts and required us to start resetting signals.  The
whole point of using sockets was to selectively ignore SIGPIPE without
modifying the signal handler.

>  	nwrite = send(fd, buf, *buf_size, MSG_NOSIGNAL);
>  
>  	if(nwrite != -1) {
> @@ -557,8 +564,11 @@ static void _alpm_reset_signals(void)
>  	int *i, signals[] = {
>  		SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, SIGILL,
>  		SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, SIGTSTP,
> -		SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, SIGTRAP,
> +		SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPROF, SIGSYS, SIGTRAP,
>  		SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
> +#ifndef __APPLE__

Signal names are macros, test for SIGPOLL directly.

> +		SIGPOLL,
> +#endif

Patch

diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index b70a8192..f7d40b78 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -34,6 +34,11 @@ 
 #include <fnmatch.h>
 #include <poll.h>
 
+/* no MSG_NOSIGNAL support */
+#ifndef MSG_NOSIGNAL
+#define MSG_NOSIGNAL 0x1022
+#endif
+
 /* libarchive */
 #include <archive.h>
 #include <archive_entry.h>
@@ -557,8 +562,11 @@  static void _alpm_reset_signals(void)
 	int *i, signals[] = {
 		SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, SIGILL,
 		SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, SIGTSTP,
-		SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, SIGTRAP,
+		SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPROF, SIGSYS, SIGTRAP,
 		SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
+#ifndef __APPLE__
+		SIGPOLL,
+#endif
 		0
 	};
 	struct sigaction def;