Re: [PATCH 1/8] More code cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 29, 2011 at 02:28:49PM +0200, Dr. Werner Fink wrote:
> >  Why? The flag in the libc struct option is always zero, so
> >  getopt_long() does not return 0. This change seems unnecessary.
> 
> This is how getopt_long() work.  It returns tzhe value if flag
> is NULL and 0 otherwise.  I'm using flag therefore if retrn 0
> here.   This is what the manual page states and I've tested.

 Sure, but this code should be in the patch where the "nipple" is
 really used. It's confusing to have "A" in one patch and "B" in
 another patch when "A" without "B" does not makes any sense.

 The final code is also inconsistent, for new (mingetty) options is
 there "nipple" pointer and for old options is there: 

     case ...:
         options->flag |= F_FOO;

 it would be better to use the same concept for all options. I prefer
 options->flag |= F_FOO ;-)

> > > +static void dolog(int priority, const char *fmt, va_list ap)
> > >  {
> > > -	va_list ap;
> > >  #ifndef	USE_SYSLOG
> > >  	int fd;
> > >  #endif
> > > @@ -1273,7 +1276,9 @@ void error(const char *fmt, ...)
> > >  	 * %m expansion is done here as well. Too bad syslog(3) does not have a
> > >  	 * vsprintf() like interface.
> > >  	 */
> > > -	va_start(ap, fmt);
> > > +#ifdef __linux__
> > > +	vsnprintf (bp, sizeof(buf)-strlen(buf), fmt, ap);
> > > +#else
> > >  	while (*fmt && bp < &buf[BUFSIZ - 1]) {
> > >  		if (strncmp(fmt, "%s", 2) == 0) {
> > >  			xstrncpy(bp, va_arg(ap, char *), &buf[BUFSIZ - 1] - bp);
> >                          ^^^^^^^
> >  You cannot remove va_start() and va_end(), you have to call in within
> >  the #else block.
> 
> Hmmm --- IMHO it can be done as I use va_start() and va_end()
> in the functions error() and warn() which are calling dolog()
> during debugging those was used very heavily ;)

 Ah, you're right. Sorry.

> > > +static void error(const char *fmt, ...)
> > > +static void warn(const char *fmt, ...)
> > 
> >   __attribute__ ((__format__ (__printf__, 1, 2)));
> 
> that is already done in the declaration at the head of agetty.c
> there is no need to tdo it twice.

 You're right.

    Karel

-- 
 Karel Zak  <kzak@xxxxxxxxxx>
 http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux