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

Attachment: signature.asc
Description: PGP signature

Reply via email to