[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: Applets send errors to syslog during normal, successful operation
From:       Tito <farmatito () tiscali ! it>
Date:       2018-02-04 23:56:20
Message-ID: 6075e43d-31f8-b677-a662-75cc0a363eee () tiscali ! it
[Download RAW message or body]

On 04/02/2018 19:53, Denys Vlasenko wrote:
> On Thu, Nov 30, 2017 at 9:51 PM, Tito <farmatito@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?!"
> 
> 
> > 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.
> 
> > I wonder if the compiler could be so smart to see that this is
> > the same as bb_error_msg when  ENABLE_FEATURE_SYSLOG is not set
> > and optimize it out.
> 
> #if !ENABLE_FEATURE_SYSLOG
> #define bb_info_msg(...) bb_error_msg(__VA_ARGS__)
> #endif
> 

Hi,
attached you will find a patch that adds bb_info_msg function to libbb.
This patch is tested with and without CONFIG_SYSLOG
set but there are no users yet.

Ciao,
Tito

--- include/libbb.h.orig	2018-02-04 23:27:22.000000000 +0100
+++ include/libbb.h	2018-02-04 23:43:08.054064117 +0100
@@ -1277,6 +1277,11 @@ extern void (*die_func)(void);
 extern void xfunc_die(void) NORETURN FAST_FUNC;
 extern void bb_show_usage(void) NORETURN FAST_FUNC;
 extern void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2))) \
FAST_FUNC; +#if ENABLE_FEATURE_SYSLOG
+extern void bb_info_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2))) \
FAST_FUNC; +#else
+#define bb_info_msg(...)	bb_error_msg(__VA_ARGS__)
+#endif
 extern void bb_error_msg_and_die(const char *s, ...) __attribute__ ((noreturn, \
format (printf, 1, 2))) FAST_FUNC;  extern void bb_perror_msg(const char *s, ...) \
__attribute__ ((format (printf, 1, 2))) FAST_FUNC;  extern void \
                bb_simple_perror_msg(const char *s) FAST_FUNC;
--- libbb/verror_msg.c.orig	2017-07-24 00:27:51.000000000 +0200
+++ libbb/verror_msg.c	2018-02-05 00:54:09.587081933 +0100
@@ -180,3 +180,17 @@ void FAST_FUNC bb_error_msg(const char *
 	bb_verror_msg(s, p, NULL);
 	va_end(p);
 }
+
+#if ENABLE_FEATURE_SYSLOG
+void FAST_FUNC bb_info_msg(const char *s, ...)
+{
+	va_list p;
+	smallint syslog_level_old =  syslog_level;
+
+	syslog_level = LOG_INFO;
+	va_start(p, s);
+	bb_verror_msg(s, p, NULL);
+	syslog_level = syslog_level_old;
+	va_end(p);
+}
+#endif


["bb_info_msg.patch" (text/x-patch)]

Add bb_info_msg function to libb. If CONFIG_SYSLOG is
set this function sends messages to the system logger
with level LOG_INFO rather than LOG_ERR as bb_error_msg.
If CONFIG_SYSLOG is not set bb_error_msg is used.

Signed-off-by: Tito Ragusa <farmatito@tiscali.it>

--- include/libbb.h.orig	2018-02-04 23:27:22.000000000 +0100
+++ include/libbb.h	2018-02-04 23:43:08.054064117 +0100
@@ -1277,6 +1277,11 @@ extern void (*die_func)(void);
 extern void xfunc_die(void) NORETURN FAST_FUNC;
 extern void bb_show_usage(void) NORETURN FAST_FUNC;
 extern void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2))) \
FAST_FUNC; +#if ENABLE_FEATURE_SYSLOG
+extern void bb_info_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2))) \
FAST_FUNC; +#else
+#define bb_info_msg(...)	bb_error_msg(__VA_ARGS__)
+#endif
 extern void bb_error_msg_and_die(const char *s, ...) __attribute__ ((noreturn, \
format (printf, 1, 2))) FAST_FUNC;  extern void bb_perror_msg(const char *s, ...) \
__attribute__ ((format (printf, 1, 2))) FAST_FUNC;  extern void \
                bb_simple_perror_msg(const char *s) FAST_FUNC;
--- libbb/verror_msg.c.orig	2017-07-24 00:27:51.000000000 +0200
+++ libbb/verror_msg.c	2018-02-05 00:54:09.587081933 +0100
@@ -180,3 +180,17 @@ void FAST_FUNC bb_error_msg(const char *
 	bb_verror_msg(s, p, NULL);
 	va_end(p);
 }
+
+#if ENABLE_FEATURE_SYSLOG
+void FAST_FUNC bb_info_msg(const char *s, ...)
+{
+	va_list p;
+	smallint syslog_level_old =  syslog_level;
+
+	syslog_level = LOG_INFO;
+	va_start(p, s);
+	bb_verror_msg(s, p, NULL);
+	syslog_level = syslog_level_old;
+	va_end(p);
+}
+#endif



_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic