Hi, On Tue, Aug 11, 2015 at 06:41:50PM +0200, David Sommerseth wrote: > From: David Sommerseth <dav...@redhat.com> > > This provides exactly the same systemd functionality which existed > before the query user infrastructure got implemented. > > [v2 - Removed the QUERY_USER_FOREACH macro]
A few nags... > + > + > +if ENABLE_SYSTEMD > +# > +# If --enable-systemd have been used, build the alternative systemd mechanism > +# The CFLAGS macro set here ensures the alternative mechanism is being used. > +# See console.h for more information. > +# > +AM_CFLAGS += -DQUERY_USER_EXEC_ALTERNATIVE > +openvpn_SOURCES += console_systemd.c > +endif Why make it so complicated? We have #ifdef ENABLE_SYSTEMD already, so we could just do this in the console.h header file... #if defined(ENABLE_SYSTEMD) || defined(MAGIC_OTHER_CONSOLE) || ... # define QUERY_USER_EXEC_ALTERNATIVE #endif IOW, we have way too many corners that spring C #define's into life, and I don't think we should invent yet a new one without very good reason - especially if that needs a long comment to explain why it's there. Thinking more about it - why not just always add console_systemd.c to openvpn_SOURCES (which, I think, is also relevant for "make dist", no?) and *inside* console_systemd.c wrap it with #if defined(ENABLE_SYSTEMD) .. #endif so it gets compiled into an empty .o - like we do with win32.c or the ssl_$library.c variants today, so it seems to work on enough platforms to not worry about that. Plus, it's "what we do elsewhere"... [..] > diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c > new file mode 100644 > index 0000000..a9cfbe6 > --- /dev/null > +++ b/src/openvpn/console_systemd.c > @@ -0,0 +1,112 @@ This looks good to me, except for the #ifdef :-) - and... > +/* > + * is systemd running > + */ > + > +static bool > +check_systemd_running () ... any particular reason for not having a (void) in there? Technically, it's "moving code without changes" (good!) but while at it, you could add this in 4/4 to be proper ANSI C :-) gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany g...@greenie.muc.de fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de
signature.asc
Description: PGP signature