[RESEND PATCH] add bb_info_msg was Re: Applets send errors to syslog during normal, successful operation

[Deweloper]

On Wed, 28 Feb 2018 14:03:46 +0100
Tito <farmatito at tiscali.it> wrote:

> Hi,
> 
> forgot to add the [PATCH] tag so I resend it.
> 
> Ciao,
> Tito
> 
> On 04/02/2018 19:53, Denys Vlasenko wrote:
> > On Thu, Nov 30, 2017 at 9:51 PM, Tito <farmatito at tiscali.it> wrote:  
> >> On 11/30/2017 08:26 PM, Deweloper wrote:  
> >>> Many applets are daemons (or can be run as daemons) and send messages to
> >>> syslog. The problem is that the messages don't have accurate, individually
> >>> assigned severity; they are all LOG_ERR. Effectively, system
> >>> administrator sees a lot of ERRORs in the log even when everything goes
> >>> well. It seems that libbb provides only bb_error_msg() as a convenient
> >>> way to print a message (including sending it to syslog), while a more
> >>> generic function taking severity as well would be needed instead. grep -r
> >>> 'syslog(' shows that only some loginutils call syslog() directly. In
> >>> other places bb_error_msg() is used even for informational or verbose
> >>> debugging messages. Just have a look at output of grep -r 'bb_error_msg('
> >>>
> >>> Do you have an idea how to clean this up? Shouldn't these messages be
> >>> sent to a new function, e.g. bb_msg(), which would additionally take
> >>> "severity" argument?  
> > 
> > The "severity" arg usually ends up being a PITA.
> > For example, it's rather unreadable.
> > Then, it tends to proliferate: after you have errors/not-errors,
> > then someone wants critical/errors/not-errors/debug - which adds
> > another dimension to coding every error message: "what severity is it?!"
> > 

Sorry, but this is how syslog() API is designed.

> >   
> >> 4) create a function that changes syslog_level to LOG_INFO, add it to
> >> libbb, and then change the code of the applets accordingly, for example:
> >>
> >> void FAST_FUNC bb_info_msg(const char *s, ...)
> >> {
> >>          va_list p;
> >> #if ENABLE_FEATURE_SYSLOG
> >>          smallint syslog_level_old =  syslog_level;
> >>          syslog_level = LOG_INFO;
> >> #endif
> >>          va_start(p, s);
> >>          bb_verror_msg(s, p, NULL);
> >> #if ENABLE_FEATURE_SYSLOG
> >>          syslog_level = syslog_level_old;
> >> #endif
> >>          va_end(p);
> >> }  
> > 
> > I like this. Feel free to send patches doing this.
> >   

As of busybox 1.28.0 the exported global variable syslog_level seems to be used only in one place @miscutils/crond.c:
> syslog_level = LOG_INFO;
> bb_verror_msg(msg, va, /* strerr: */ NULL);
> syslog_level = LOG_ERR;
IMO it would be much better to completely get rid of it and - surprise - use function parameters for passing parameters to functions.

--