Message ID | 20230920121519.177949-1-frank@lichtenheld.com |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel] CMake: fix broken daemonization and syslog functionality | expand |
On Wed, Sep 20, 2023 at 02:15:19PM +0200, Frank Lichtenheld wrote: > From: Lev Stipakov <lev@openvpn.net> > > While CMake is not the official way to build OpenVPN on Linux, > it still make sense to support it. Turns out that > > HAVE_SETSID, HAVE_OPENLOG and HAVE_SYSLOG > > were not set by CMake configure, and --daemon and syslog > functionality was broken. > > While on it, fix compiler error on unused return value of chdir(). > > Change-Id: I171d55da2be868d961caa1d4491e6f1ed10ebe8a > Signed-off-by: Lev Stipakov <lev@openvpn.net> > Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org> > Acked-by: Frank Lichtenheld <frank@lichtenheld.com> > --- > > This change was reviewed on Gerrit and approved by at least one > developer. I request to merge it to master. > > Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/341 > This mail reflects revision 1 of this Change. > Acked-by according to Gerrit (reflected above): > Arne Schwabe <arne-openvpn@rfc2549.org> > Frank Lichtenheld <frank@lichtenheld.com> So I can automatically set In-Reply-To to the same "virtual" message ID that Gerrit uses (since that is deterministally constructed from the change metadata). However, since this is not the actual message ID by the original "newchange" mail, it still doesn't associate to the same "Patch" in Patchwork. Instead it creates a new patch. Regards,
Your patch has been applied to the master branch. I have not tested it further, given that it has plenty of ACKs (and only affects cmake build system). Two observations, though - the cmake changes look incomplete, there is still many "#undef HAVE_something" left. Shouldn't they all be completed to "#cmakedefine" for this to make sense? Second, while not really relevant on Linux or *BSD, erroring-out in daemon() on chdir("/") fail is not what either manpage documents - and I do not think chdir("/") (as root) can ever fail. So changing the code flow just to silence the compiler feels a bit meh - doing a (void) there would have silenced it just fine. But as I said it's not truly relevant. What OS were you building anyway that you hit an error in compat-daemon.c (aka "are we missing a cmake test for daemon()")? commit 8ae6c48d5d52dec8ec6e47cc1cfe89de9f2ffbcd (master) Author: Lev Stipakov Date: Wed Sep 20 14:15:19 2023 +0200 CMake: fix broken daemonization and syslog functionality Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com> Message-Id: <20230920121519.177949-1-frank@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27045.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/CMakeLists.txt b/CMakeLists.txt index 7dae665..9de6aba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -150,6 +150,7 @@ check_symbol_exists(ftruncate unistd.h HAVE_FTRUNCATE) check_symbol_exists(setgid unistd.h HAVE_SETGID) check_symbol_exists(setuid unistd.h HAVE_SETUID) +check_symbol_exists(setsid unistd.h HAVE_SETSID) check_symbol_exists(getpeereid unistd.h HAVE_GETPEEREID) check_symbol_exists(epoll_create sys/epoll.h HAVE_EPOLL_CREATE) diff --git a/config.h.cmake.in b/config.h.cmake.in index 4f28917..f2cdd39 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -218,7 +218,7 @@ #undef HAVE_NICE /* Define to 1 if you have the `openlog' function. */ -#undef HAVE_OPENLOG +#cmakedefine HAVE_OPENLOG /* OpenSSL engine support available */ #undef HAVE_OPENSSL_ENGINE @@ -258,7 +258,7 @@ #undef HAVE_SETGROUPS /* Define to 1 if you have the `setsid' function. */ -#undef HAVE_SETSID +#cmakedefine HAVE_SETSID /* Define to 1 if you have the `setsockopt' function. */ #define HAVE_SETSOCKOPT 1 @@ -303,7 +303,7 @@ #undef HAVE_STRSEP /* Define to 1 if you have the `syslog' function. */ -#undef HAVE_SYSLOG +#cmakedefine HAVE_SYSLOG /* Define to 1 if you have the <syslog.h> header file. */ #cmakedefine HAVE_SYSLOG_H diff --git a/src/compat/compat-daemon.c b/src/compat/compat-daemon.c index aebb8f4..5c6d740 100644 --- a/src/compat/compat-daemon.c +++ b/src/compat/compat-daemon.c @@ -70,7 +70,10 @@ if (!nochdir) { - chdir("/"); + if (chdir("/") == -1) + { + return (-1); + } } if (!noclose)