Eric Blake <ebl...@redhat.com> wrote on 09/24/2010 04:01:55 PM:

> libvir-list
> 
> On 09/24/2010 01:38 PM, Stefan Berger wrote:
> 
> > To prevent consecutive spaces in comments from becoming a single space
> > (by bash), the IFS variable is now set to an empty string. Also, 
commands
> > are now executed using bash's 'eval' command.
> 
> -#define CMD_EXEC   "res=`${cmd}`" CMD_SEPARATOR
> +#define CMD_EXEC   "res=`eval ${cmd}`" CMD_SEPARATOR
> 
> Underquoted.  To be robust, this needs to be:
> 
> "res=`eval \"$cmd\"" CMD_SEPARATOR
> 

Ok, I made this change.

> which will then avoid your need for the empty IFS hack and double 
> escaping (you'll still need single escaping, but that's a bit more 
> manageable).

I just tried the TCK test without and with double-escaping in libvirtd
and double-escaping does seem to be necessary otherwise `ls` and $(ls)
do get executed and their results end up in the comment. The spaces
are preserved, though, so I can revert the change to IFS.

> 
> > +/* avoiding a compiler warning trough own implementation */
> > +static const char *
> > +_strchr(const char *s, int c)
> > +{
> > +    while (*s && *s != (char)c)
> > +        s++;
> > +    if (*s)
> > +        return s;
> > +    return NULL;
> > +}
> >
> 
> Ouch.  That's probably 4x slower than the glibc version.  I'd much 
> rather see:
> 
> #undef strchr
> 

Yes, that does the trick. Thanks.

> or
> 
> (strchr)(a, b)
> 
> which then guarantees that you get the function call, rather than the 
> macro expansion; after all, the macro expansion just defers to the 
> function call if both arguments are non-constants, not to mention the 
> fact that the -Wlogical-op warning is only triggered by the macro and 
> not by the function.

  Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to