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

List:       busybox
Subject:    Re: [PATCH 3/7] klogd: support /dev/klog in addition to klogctl()
From:       Jeremie Koenig <jk () jk ! fr ! eu ! org>
Date:       2010-07-30 19:09:34
Message-ID: 20100730190934.GA1566 () arabica
[Download RAW message or body]

On Fri, Jul 30, 2010 at 03:18:47AM +0200, Denys Vlasenko wrote:
> On Thursday 29 July 2010 04:29, Jeremie Koenig wrote:
> > +/* FIXME: consumes global static memory */
> > +static int klogfd = 0;
> 
> Use fixed fd == 3 in order to not use globals:

Okay, thanks.

> > +	if (fd < 0) {
> > +		syslog(LOG_ERR, "klogd: can't open "_PATH_KLOG" (error %d: %m)",
> > +				errno);
> > +		exit(EXIT_FAILURE);
>      ^^^^^^^^^ "logmode = SYSLOG; bb_perror_msg_and_die(...)" will be smaller.

Okay, thanks.

> Just be sure to call klogd_open() before openlog(), since
> openlog() may use fd 3 otherwise!

I used fd 4 instead, since klogd_open() may need to log the above error.
Another possibility would be to write the message on stderr instead of
the log.

> > +static inline void klogd_close(void)
> 
> Do not inline use-once functions, gcc will do it for you where needed.

Okay, thanks, I removed all inlines.

> > +static inline void klogd_setloglevel(int lvl UNUSED_PARAM)
> > +{
> > +	syslog(LOG_WARNING, "klogd warning: this build does not support"
> > +			" changing the console log level");
> 
> Can you use /proc/sys/kernel/printk for it?

Sure, I also called klogd_setloglevel() from klogd_close() and updated
the configuration help message.

I'll post the new patch after some testing.

-- 
Jérémie Koenig <jk@jk.fr.eu.org>
_______________________________________________
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]