[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 29 Aug 2016 05:40:52 +0000
From: Noam Meltzer <tsnoam@...il.com>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix strdupa evaulating expression twice
On Sun, Aug 28, 2016 at 8:04 PM Bobby Bingham <koorogi@...rogi.info> wrote:
> On Sun, Aug 28, 2016 at 04:53:50PM +0000, Noam Meltzer wrote:
> > On Sun, Aug 28, 2016 at 6:08 PM Rich Felker <dalias@...c.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@...il.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.
>
> _GNU_SOURCE says that the program has requested glibc extensions, like
> strdupa. It does not imply anything about the compiler being used and
> whether it implements GCC extensions.
>
Thanks for clearing that out.
>
> That said, it looks like glibc only defines strdupa #if __GNUC__, so
> this function is simply unavailable when using a compiler that doesn't
> implement this extension.
>
> I'm not sure what the musl policy is here, but maybe we could do
> something similar?
>
The __GNUC__ macros is being used in other parts of musl, so is there a
reason not to move strdupa into a #if __GNUC__ macro?
> >
> > 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
>
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
