Hi, Sorry to bring those suggestions below a bit late...
----- David Maciejak <[email protected]> a écrit : > [...] > > diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h > index 246ef2d..b2e76ab 100644 > --- a/WINGs/WINGs/WUtil.h > +++ b/WINGs/WINGs/WUtil.h > @@ -243,6 +243,9 @@ enum { > > +void syslog_shutdown(void); The file WUtil.h contains a public API, so it should be changed with care, especially concerning the consistency in function names. May I suggest: - to start the function name with a 'w' like the other functions? - to not make a reference to 'syslog', as I feel the WUtil library should be hiding these internals to end users? I would propose something like "wfinish" or "wshutdown" which are more neutral and could include more things in the future if we need to. > diff --git a/WINGs/error.c b/WINGs/error.c > index 2d5a588..d966d99 100644 > --- a/WINGs/error.c > +++ b/WINGs/error.c > @@ -30,6 +30,46 @@ > +void syslog_open(char *prog_name) > +void syslog_message(int prio, char *prog_name, char *msg) Shouldn't these be 'static' ? it's not part of the API but local to that file, right? Also, as they are called only when HAVE_SYSLOG is defined, you should probably enclose the complete functions in #ifdef, not just their bodies, it would make the code shorter. > @@ -86,6 +142,9 @@ void __wmessage(const char *func, const char *file, > int line, int type, const ch > va_end(args); > > fputs(buf, stderr); > +#ifdef HAVE_SYSLOG > + syslog_message(syslog_priority, _WINGS_progname ? _WINGS_progname : > "WINGs", buf); > +#endif It looks like the message is always printed in stderr and in syslog; I don't what the traditional behaviour is but shouldn't we propose an environment variable to allow dynamic choice for either stderr or syslog or both? > diff --git a/src/startup.c b/src/startup.c > @@ -160,6 +160,11 @@ static RETSIGTYPE handleExitSig(int sig) > > sigprocmask(SIG_UNBLOCK, &sigs, NULL); > + > +#ifdef HAVE_SYSLOG > + syslog_shutdown(); > +#endif > + Considering the function is defined as empty when HAVE_SYSLOG is not defined, I would suggest to always call it here (unconditionally) and let the function do nothing when syslog is not used. And linked to the remark at the beginning, this could be useful in the future. -- To unsubscribe, send mail to [email protected].
