dbus user existance checking.

Pat Suwalski pat at suwalski.net
Fri Apr 16 11:48:17 EST 2004


Havoc Pennington wrote:
> Hi,
> 
> Thanks for the patch, some comments:
> 
>  1. I think we want a syslog=yes|no option in the config file, and 
>     default yes for system daemon and no for session; my experience 
>     from gconf is that admins aren't big fans of anything in syslog
>     unless said thing requires their intervention.

That is true. I think there should be an option, and I'll look into 
implementing it, but I have a few other ideas as well...

<snip>

>     The problem after that is how to get the setting for each 
>     BusContext to _dbus_warn and it probably means adding 
>     a _dbus_warn_with_log(dbus_bool_t log, const char *format, )
>     function or something like that then fixing up all the 
>     _dbus_warn to honor the flag in the BusContext, if that 
>     makes sense.
> 
>  2. So further along those lines, we sort of need to grep for 
>     _dbus_warn and see how many of them would be irritating if 
>     they ended up in syslog

Combining these two points: after grepping the source for _dbus_warn, 
that function does way too much. If all the code has to change how 
warnings are called anyway, why add a _warn_with_log?

My observations with the _warn function is that it's used way outside of 
its scope. To me, a warning should be non-fatal and potentially loggable 
(as per configuration). Right now, it is used equally often for warnings 
(correct) and for fatal errors (incorrect, IMO). If, due to the addition 
of logging options, the function were split into two functions, _warn 
and _error, it would make more sense. Then fatal things, which 
definitely belong in the log could be inserted there from the error 
function. _error could simply call _warn(log=true) or whatnot.

It might take some care as to which current warnings to move to errors, 
but is it not expected system daemon behaviour to log fatals?

>  3. The configure.in check isn't quite right; the line:
>     AC_CHECK_HEADERS(execinfo.h, [AC_CHECK_FUNCS(backtrace)])
>     is checking for the execinfo.h header, and if that's found 
>     checking for the backtrace function. This is then used 
>     in dbus-sysdeps.c:
>       #if defined (HAVE_BACKTRACE)
> 
>     So the vsyslog check has to be separate from the backtrace 
>     check, and it's pointless unless we use HAVE_VSYSLOG in the 
>     code somewhere.

I see. This is simple enough to fix. I was not aware that the header 
check was being used as a conditional setting function.

Thanks.
--Pat



More information about the dbus mailing list