thx Christophe for the comments, as i said previously, that's draft
code I mean it works well but can be enhanced will the suggestions of
everybody.
It could be great if you could implement your suggestions in a patch.
regards,
david
On Sun, Feb 23, 2014 at 11:51 AM, Christophe <christophe.cu...@free.fr> wrote:
> Hi,
>
> Sorry to bring those suggestions below a bit late...
>
>
> ----- David Maciejak <david.macie...@gmail.com> 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 wmaker-dev-unsubscr...@lists.windowmaker.org.
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
