[tarantool-patches] Re: [PATCH] Enable support for NOTIFY_SOCKET on macOS

["Maxim Melentiev" <dmarc-noreply@freelists.org> (Redacted sender "m.melentiev" for DMARC)]

 From d171f4a0721be0168e591aaa734c4729b96ea87c Mon Sep 17 00:00:00 2001
From: Max Melentiev <m.melentiev@corp.mail.ru>
Date: Thu, 25 Jul 2019 10:05:24 +0300
Subject: [PATCH] Enable support for NOTIFY_SOCKET in envs without systemd

To make it possible to develop and test related features on
systems without systemd.

WITH_SYSTEMD cmake flag is used to generate systemd related files:
unit, generator script, etc. To keep this behavior and make it possible
to use NOTIFY_SOCKET without other systemd-related stuff,
I added WITH_NOTIFY_SOCKET cmake flag.

It also required some changes to support other OS:

SOCK_CLOEXEC (not available on macOS) flag for socket()
is replaced with `fcntl(fd, F_SETFD, FD_CLOEXEC)` which has the same effect.

MSG_NOSIGNAL flag for sendmsg is also not available on macOS.
However it has SO_NOSIGPIPE flag for setsockopt which disables SIGPIPE.
So it requires different solution for different OS. Inspired by
https://nwat.xyz/blog/2014/01/16/porting-msg_more-and-msg_nosigpipe-to-osx/

Have to reduce send-buffer size to 4MB because larger values
are not supported on macOS by default. This value should be enough
for all systems because notification messages are usually less than 1KB.

Fixes #4436
---
  CMakeLists.txt            | 28 +++++++++++++++++++++++++++-
  src/systemd.c             | 28 +++++++++++++++++++++++-----
  src/systemd.h             |  6 +++---
  src/trivia/config.h.cmake |  7 +++++--
  4 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index bfb15effb..1aef3a4d2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -282,7 +282,6 @@ find_package_message(MODULE_LUAPATH "Lua 
package.path: ${MODULE_LUAPATH}"
  find_package_message(MODULE_LIBPATH "Lua package.cpath: ${MODULE_LIBPATH}"
      "${MODULE_LIBPATH}")

-
  ##
  ## Third-Party libraries
  ##
@@ -482,6 +481,33 @@ add_subdirectory(extra)
  add_subdirectory(test)
  add_subdirectory(doc)

+option(WITH_NOTIFY_SOCKET "Enable notifications on NOTIFY_SOCKET" ON)
+
+if (WITH_SYSTEMD AND NOT WITH_NOTIFY_SOCKET)
+    message(FATAL_ERROR "WITH_NOTIFY_SOCKET must be enabled when 
WITH_SYSTEMD enabled")
+endif()
+
+if (WITH_NOTIFY_SOCKET)
+    check_c_source_compiles("
+        #include <sys/types.h>
+        #include <sys/socket.h>
+        int main(){ return MSG_NOSIGNAL; }
+    " HAVE_MSG_NOSIGNAL)
+    check_c_source_compiles("
+        #include <sys/types.h>
+        #include <sys/socket.h>
+        int main(){ return SO_NOSIGPIPE; }
+    " HAVE_SO_NOSIGPIPE)
+    # Linux supports MSG_NOSIGNAL flag for sendmsg.
+    # macOS lacks it, but has SO_NOSIGPIPE for setsockopt
+    # to achieve same behavior.
+    if (NOT HAVE_MSG_NOSIGNAL AND NOT HAVE_SO_NOSIGPIPE)
+        message(FATAL_ERROR
+            "No way to block SIGPIPE in sendmsg. Can not compile with 
WITH_NOTIFY_SOCKET"
+        )
+    endif()
+endif()
+
  if(NOT "${PROJECT_BINARY_DIR}" STREQUAL "${PROJECT_SOURCE_DIR}")
      add_custom_target(distclean)
      add_custom_command(TARGET distclean
diff --git a/src/systemd.c b/src/systemd.c
index 073c9bb76..0cc929274 100644
--- a/src/systemd.c
+++ b/src/systemd.c
@@ -30,7 +30,7 @@
   */
  #include "systemd.h"

-#if defined(WITH_SYSTEMD)
+#if defined(WITH_NOTIFY_SOCKET)
  #include <errno.h>
  #include <stdio.h>
  #include <stdarg.h>
@@ -68,11 +68,25 @@ int systemd_init() {
          say_error("systemd: NOTIFY_SOCKET is longer that MAX_UNIX_PATH");
          goto error;
      }
-    if ((systemd_fd = socket(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0)) == 
-1) {
+    if ((systemd_fd = socket(AF_UNIX, SOCK_DGRAM, 0)) == -1) {
          say_syserror("systemd: failed to create unix socket");
          goto error;
      }
-    int sndbuf_sz = 8 * 1024 * 1024;
+    if (fcntl(systemd_fd, F_SETFD, FD_CLOEXEC) == -1) {
+        say_syserror("systemd: fcntl failed to set FD_CLOEXEC");
+        goto error;
+    }
+
+    #ifdef HAVE_SO_NOSIGPIPE
+    int val = 1;
+    if (setsockopt(systemd_fd, SOL_SOCKET, SO_NOSIGPIPE, (void*)&val,
+            sizeof(val)) < 0) {
+        say_syserror("systemd: failed to set NOSIGPIPE");
+        goto error;
+    }
+    #endif
+
+    int sndbuf_sz = 4 * 1024 * 1024;
      if (setsockopt(systemd_fd, SOL_SOCKET, SO_SNDBUF, &sndbuf_sz,
                sizeof(int)) < 0) {
          say_syserror("systemd: failed to set sndbuf size");
@@ -106,8 +120,12 @@ int systemd_notify(const char *message) {
          sa.sun_path[0] = '\0';

      say_debug("systemd: sending message '%s'", message);
+    int flags = 0;
+    #ifdef HAVE_MSG_NOSIGNAL
+        flags |= MSG_NOSIGNAL;
+    #endif
      ssize_t sent = sendto(systemd_fd, message, (size_t) strlen(message),
-        MSG_NOSIGNAL, (struct sockaddr *) &sa, sizeof(sa));
+        flags, (struct sockaddr *) &sa, sizeof(sa));
      if (sent == -1) {
          say_syserror("systemd: failed to send message");
          return -1;
@@ -142,4 +160,4 @@ systemd_snotify(const char *format, ...)
      va_end(args);
      return res;
  }
-#endif /* defined(WITH_SYSTEMD) */
+#endif /* defined(WITH_NOTIFY_SOCKET) */
diff --git a/src/systemd.h b/src/systemd.h
index f4f36e2cf..861a9af35 100644
--- a/src/systemd.h
+++ b/src/systemd.h
@@ -39,7 +39,7 @@
  extern "C" {
  #endif /* defined(__cplusplus) */

-#if defined(WITH_SYSTEMD)
+#if defined(WITH_NOTIFY_SOCKET)
  /**
   * Open connection with systemd daemon (using unix socket located in
   * "NOTIFY_SOCKET" environmnent variable)
@@ -95,13 +95,13 @@ systemd_vsnotify(const char *format, va_list ap);
  int
  systemd_snotify(const char *format, ...);

-#else /* !defined(WITH_SYSTEMD) */
+#else /* !defined(WITH_NOTIFY_SOCKET) */
  #  define systemd_init()
  #  define systemd_free()
  #  define systemd_notify(...)
  #  define systemd_vsnotify(...)
  #  define systemd_snotify(...)
-#endif /* WITH_SYSTEMD */
+#endif /* WITH_NOTIFY_SOCKET */

  #if defined(__cplusplus)
  } /* extern "C" */
diff --git a/src/trivia/config.h.cmake b/src/trivia/config.h.cmake
index ca0057d2b..226f83128 100644
--- a/src/trivia/config.h.cmake
+++ b/src/trivia/config.h.cmake
@@ -171,6 +171,9 @@
  #cmakedefine HAVE_MREMAP 1
  #cmakedefine HAVE_SYNC_FILE_RANGE 1

+#cmakedefine HAVE_MSG_NOSIGNAL 1
+#cmakedefine HAVE_SO_NOSIGPIPE 1
+
  #cmakedefine HAVE_PRCTL_H 1

  #cmakedefine HAVE_UUIDGEN 1
@@ -202,9 +205,9 @@
  #cmakedefine HAVE_ICU_STRCOLLUTF8 1

  /*
-* Defined if systemd is enabled
+* Defined if notifications on NOTIFY_SOCKET are enabled
   */
-#cmakedefine WITH_SYSTEMD 1
+#cmakedefine WITH_NOTIFY_SOCKET 1

  /** \cond public */

--
2.21.0

On 22/08/2019 13:06, Konstantin Osipov wrote:
> * Maxim Melentiev <m.melentiev@corp.mail.ru> [19/08/22 12:04]:
>> And C code uses #ifdef SO_NOSIGPIP, #ifdef MSG_NOSIGNAL
> C code should use config.h defines, HAVE_MSG_NOSIGNAL,
> HAVE_SO_NOSIGPIPE.
>