Eric Blake <eblake@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