Re: [PATCH] fix strdupa evaulating expression twice

[Noam Meltzer <tsnoam@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

On Sun, Aug 28, 2016 at 6:08 PM Rich Felker <dalias@libc.org> wrote:

> On Sun, Aug 28, 2016 at 10:59:26AM +0000, Noam Meltzer wrote:
> >
>
> > From bf23b7b8fd39eaca6a05173eaf543e1bce3319ab Mon Sep 17 00:00:00 2001
> > From: Noam Meltzer <tsnoam@gmail.com>
> > Date: Sun, 28 Aug 2016 13:53:24 +0300
> > Subject: [PATCH] fix strdupa evaulating expression twice
> >
> > calling strdupa with va_arg as its expression caused unexpected
> > behaviour. now the expression is evaulated only once.
> > ---
> >  include/string.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/string.h b/include/string.h
> > index ff9badb..976faaf 100644
> > --- a/include/string.h
> > +++ b/include/string.h
> > @@ -85,7 +85,10 @@ size_t strlcpy (char *, const char *, size_t);
> >  #endif
> >
> >  #ifdef _GNU_SOURCE
> > -#define      strdupa(x)      strcpy(alloca(strlen(x)+1),x)
> > +#define      strdupa(x) (__extension__ ({ \
> > +             const char *__xval = x; \
> > +             strcpy(alloca(strlen(__xval)+1),__xval); \
> > +     }))
> >  int strverscmp (const char *, const char *);
> >  int strcasecmp_l (const char *, const char *, locale_t);
> >  int strncasecmp_l (const char *, const char *, size_t, locale_t);
>
> The intent of the form as written is to be actual C (modulo use of
> alloca) rather than "GNU C". Aside from that, strdupa is essentially
> always-unsafe and should probably be removed or at least made into a
> warning...
>

I understand what you're saying and I tend to agree. However:
 * The entire section of the code is wrapped with the _GNU_SOURCE test
macro (so the __extension__ trick should work).
 * IMHO, if strdupa is to kept, it should at least provide expected
behaviour.

p.s.
I spent about a day of work chasing a bug caused by the current
implementation. So what I actually looking for is to save this pain from
others one way or another.

 - Noam

[-- Attachment #2: Type: text/html, Size: 2581 bytes --]