[PATCH] new bb_msg_and_syslog + sulogin clean up.

Tito farmatito at tiscali.it
Fri Aug 25 12:57:01 UTC 2006


On Friday 25 August 2006 00:05, you wrote:
> Having just received your resend, here's what I had to say the first time:
> 
> On Thursday 20 July 2006 4:47 pm, Tito wrote:
> > Hi,
> > this set of patches (2) contains a new bb_msg_and_syslog
> > function for libbb  and shows the use of this new function in the sulogin
> > applet.
> 
> I think this would be conceptually cleaner as three functions:
> 
> write_syslog() - just does the syslog
> perror_syslog() - syslog and stderr
> perror_syslog_and_die() - syslog and stderr, then die.

Hi,
Yes I've thought about this too, but my personal taste is
to prefer one flexible function that can be used in the most common
cases by changing the behaviour through some kind of flags,
like in this case trough the level (a parameter we have to pass to the function
anyway)

                       if level is:  LOG_INFO, LOG_NOTICE 
                                                       the message will not contain the applet's name and 
                                                       goes to stdout;
                       if level is:  LOG_DEBUG (only if CONFIG_DEBUG is set), LOG_WARNING 
                                                       the message will contain the applet's name and 
                                                       goes to stderr;
                       if level is:  LOG_ERR,  LOG_CRIT, LOG_ALERT, LOG_EMERG 
                                                       the message will contain the applet's name and 
                                                       goes to stderr, after printing this message 
                                                       the progam will exit with a return value of EXIT_FAILURE.

Feel free to change it ....or let me know how exactly it should work and i could try
to reimplement it.

BTW: i was thinking to eliminate the bb_facility global var by passing it also
as part of the first parameter to msg_and_syslog:

int bb_facility = LOG_USER;

void bb_msg_and_syslog(int level, const char *fmt, ...)

for example like this:
void bb_msg_and_syslog((LOG_USER*1000)+LOG_ONLY+LOG_ERR+LOG_DIE, const char *fmt, ...)

and parse it later. But as this var is set just one time per app maybe it is not worth the effort........

Optionally also a LOG_DIE flag could be added to make it possible
to define with a better granularity when we want the app to die or not.... 

> In theory each lower one is a wrapper around a higher one, but I'm not quite 
> sure how to do the va_stuff handoff to get them both to work like printf.

Maybe something like this?!

From man vprintf:
      To allocate a sufficiently large string and print into it (code correct
       for both glibc 2.0 and glibc 2.1):
              #include <stdio.h>
              #include <stdlib.h>
              #include <stdarg.h>

              char *
              make_message(const char *fmt, ...) {
                 /* Guess we need no more than 100 bytes. */
                 int n, size = 100;
                 char *p, *np;
                 va_list ap;

                 if ((p = malloc (size)) == NULL)
                    return NULL;

                 while (1) {
                    /* Try to print in the allocated space. */
                    va_start(ap, fmt);
                    n = vsnprintf (p, size, fmt, ap);
                    va_end(ap);
                    /* If that worked, return the string. */
                    if (n > -1 && n < size)
                       return p;
                    /* Else try again with more space. */
                    if (n > -1)    /* glibc 2.1 */
                       size = n+1; /* precisely what is needed */
                    else           /* glibc 2.0 */
                       size *= 2;  /* twice the old size */
                    if ((np = realloc (p, size)) == NULL) {
                       free(p);
                       return NULL;
                    } else {
                       p = np;
                    }
                 }
              }

and then use p to pass it to syslog or printf or bb_error_msg..etc.

> Any debug_syslog() thing should be a macro that optimizes out if ENABLE_DEBUG 
> is 0.  I don't think that's really required, though.

Yes, it is just there to rip out the check for       LOG_DEBUG
as it is unlikely to be found when ENABLE DEBUG is not set

USE_DEBUG(case LOG_DEBUG:)

> Closing the fd only if FEATURE_CLEAN_UP is set strikes me as dangerous.  What 
> if dnsd or httpd starts using this?  The leaks can add up and get nasty after 
> a while.

Just followed the man:
closelog() closes the descriptor being used to write to the system logger.  The use of closelog() is optional.
Will fix it.

> Hmmm...  I've been pondering consolidating show_usage(), error_msg(), 
> error_msg_and_die(), perror_msg(), perror_msg_and_die(), vherror_msg(), 
> herror_msg(), herror_msg_and_die(), perror_nomsg_and_die(), perror_nomsg(), 
> verror_msg(), and vperror_msg().  They should all be in one .c file (ala 
> llist.c or xfuncs.c).  This belongs in with that pile, although I won't be 
> able to do that consolidation before this weekend.
> 
> I'm also likely to remove the bb_ prefixes at the same time because I really 
> don't like them. 

Could rename it to msg_and_syslog.

> Nothing in libc has a prefix saying it's part of libc.  In  
> zlib.h there are functions called inflateInit(), uncompress(), and adler32(), 
> crc32(), typedefs called alloc_func and free_func...  You can see the FSF try 
> to do these kind of prefixes in bfd.h, but at the same time typedef carsym 
> and alent and such with no prefixes...  Our stuff is _not designed to be 
> added randomly to third party programs, and the bb_ prefix for _ourselves_ is 
> just silly...
> 
> The documentation says LOG_QUIET but the code says LOG_ONLY...

Yes have to fix it to LOG_ONLY as in libbb.h.

> My fiancee is dragging me away to dinner.  More later...
> 
> Rob
Ciao,
Tito



More information about the busybox mailing list