[PATCH] syslogd: convert dummy functions to statics and get rid of IF_FEATURE_* checks

Mike Frysinger vapier at gentoo.org
Sun Jan 6 18:38:29 UTC 2013


On Sunday 06 January 2013 08:17:13 Tito wrote:
> On Sunday 06 January 2013 13:11:04 Peter Korsgaard wrote:
> > As suggested by Mike. No bloat-o-meter difference, but a bit nicer to
> > look at. We cannot convert the call to log_to_shmem() as it checks for
> > G.shbuf outside the function, and G.shbuf is only available when IPC
> > support is enabled.
> > 
> > --- a/sysklogd/syslogd.c
> > +++ b/sysklogd/syslogd.c
> > @@ -529,8 +529,8 @@ static void log_to_shmem(const char *msg)
> >  		printf("tail:%d\n", G.shbuf->tail);
> >  }
> >  #else
> > -void ipcsyslog_cleanup(void);
> > -void ipcsyslog_init(void);
> > +static void ipcsyslog_cleanup(void) {}
> > +static void ipcsyslog_init(void) {}
> >  void log_to_shmem(const char *msg);
> >  #endif /* FEATURE_IPC_SYSLOG */
> > @@ -567,9 +567,9 @@ static void log_to_kmsg(int pri, const char *msg)
> >  	write(G.kmsgfd, G.printbuf, sprintf(G.printbuf, "<%d>%s\n", pri, 
msg));
> >  }
> >  #else
> > -void kmsg_init(void);
> > -void kmsg_cleanup(void);
> > -void log_to_kmsg(int pri, const char *msg);
> > +static void kmsg_init(void) {}
> > +static void kmsg_cleanup(void) {}
> > +static void log_to_kmsg(int pri UNUSED_PARAM, const char *msg
> > UNUSED_PARAM) {}
> >  #endif /* FEATURE_KMSG_SYSLOG */
> >  /* Print a message to the log file. */
> > @@ -706,7 +706,7 @@ static void timestamp_and_log(int pri, char *msg, int
> > len)
> >  	}
> >  	timestamp[15] = '\0';
> > -	if (ENABLE_FEATURE_KMSG_SYSLOG && (option_mask32 & OPT_kmsg)) {
> > +	if (option_mask32 & OPT_kmsg) {
> >  		log_to_kmsg(pri, msg);
> >  		return;
> >  	}
> > @@ -881,11 +881,10 @@ static void do_syslogd(void)
> >  #endif
> >  	sock_fd = create_socket();
> > -	if (ENABLE_FEATURE_IPC_SYSLOG && (option_mask32 & OPT_circularlog)) {
> > +	if (option_mask32 & OPT_circularlog)
> >  		ipcsyslog_init();
> > -	}
> > -	if (ENABLE_FEATURE_KMSG_SYSLOG && (option_mask32 & OPT_kmsg))
> > +	if (option_mask32 & OPT_kmsg)
> >  		kmsg_init();
> >  	timestamp_and_log_internal("syslogd started: BusyBox v" BB_VER);
> > @@ -974,9 +973,8 @@ static void do_syslogd(void)
> >  	timestamp_and_log_internal("syslogd exiting");
> >  	puts("syslogd exiting");
> >  	remove_pidfile(CONFIG_PID_FILE_PATH "/syslogd.pid");
> > -	if (ENABLE_FEATURE_IPC_SYSLOG)
> > -		ipcsyslog_cleanup();
> > -	if (ENABLE_FEATURE_KMSG_SYSLOG && (option_mask32 & OPT_kmsg))
> > +	ipcsyslog_cleanup();
> > +	if (option_mask32 & OPT_kmsg)
> >  		kmsg_cleanup();
> >  	kill_myself_with_sig(bb_got_signal);
> >  #undef recvbuf
> 
> the previous version makes the code more readable,
> now you have to know about the ENABLE_FEATURE__XXX
> or search the code to find the #ifdefs
> to guess why a function is not executed.

i don't think so.  if you look at where the funcs are defined, when the feature 
is disabled, there are stubs that do nothing.  that means the calling code 
simply has to worry about initializing & cleaning up the feature (which it 
already has to do), not whether the feature is available.

let's see if Denys has an opinion.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20130106/3c450001/attachment-0001.asc>


More information about the busybox mailing list