On 28/02/2018 18:27, Deweloper wrote:
On Wed, 28 Feb 2018 14:03:46 +0100
Tito <[email protected]> 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 <[email protected]> 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.
Hi,
I think that it is not that practical to add one more parameter to all
bb_error_msg and bb_error_msg_and_die calls because:
1) they are error messaging functions and LOG_ERR is mostly right
2) if ENABLE_FEATURE_SYSLOG is not set they don't do any syslog
messaging at all and the parameter would be superfluous.
3) it will increase space
4) syslog_level allows already to to handle exceptions (set to LOG_INFO
or other LOG_...)
5) bb_info_msg is a commodity function of 4) to better tune
syslog output by using existing machinery of libbb
without "uglyfying" the code with syslog_level = LOG_INFO
and syslog_level = LOG_ERR line pairs and it
is automagically converted to bb_error_msg if
ENABLE_FEATURE_SYSLOG is not set, needs no extra args and
should not increase binary size that much.
Ciao,
Tito
BTW: miscutils/crond.c: seems the perfect place for using bb_info_msg.
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
