[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 22 May 2012 16:19:31 -0700
From: Isaac Dunham <idunham@...abit.com>
To: musl@...ts.openwall.com
Subject: Re: [PATCH]_BSD_SOURCE for musl, take 2.
On Tue, 22 May 2012 11:22:56 -0400
Rich Felker <dalias@...ifal.cx> wrote:
> On Mon, May 21, 2012 at 11:31:58AM -0700, Isaac Dunham wrote:
> > Here's a second verion of the patch, with all the issues below
> > addressed.
>
> Looks good. A few more things, mostly just questions to make sure
> stuff is ok..
>
> > I'm still declaring bsd_signal if _BSD_SOURCE is defined, because it
> > uses the signature of BSD signal().
>
> Is there a reason for this? bsd_signal was a legacy SUSv3 function
> removed in SUSv4/POSIX-2008, which was originally added mainly so that
> programs written for BSD signal semantics could do:
>
> #define signal bsd_signal
>
> on non-BSD systems and still work. On glibc and musl it's utterly
> useless since signal, by default, has BSD semantics. (POSIX allows
> either and only BSD is sane.)
> I doubt any code for BSD would ever be calling bsd_signal, as the
> whole purpose of it is to provide an alternate function with BSD
> behavior on non-BSD systems.
I had not realized that musl used BSD semantics. You can drop that, if
you want.
> > diff --git a/include/netinet/tcp.h b/include/netinet/tcp.h
> > index c8a1a4b..797ce68 100644
> > --- a/include/netinet/tcp.h
> > +++ b/include/netinet/tcp.h
> > @@ -2,5 +2,22 @@
> > #define _NETINET_TCP_H
> >
> > #define TCP_NODELAY 1
> > +#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> > +#include <sys/types.h>
> > +#include <sys/socket.h>
> > +#define TCP_MAXSEG 2
> > +#define TCP_CORK 3
> > +#define TCP_KEEPIDLE 4
> > [...]
>
> TCP_* is in the reserved namespace for this header, so I think it
> would be reasonable to define all the macros unconditionally. What
> about the two includes, though? If they're needed to make GNU/BSD
> programs work, we should still keep the check just for them.
I'm not sure whether the includes are expected by some programs.
> > diff --git a/include/unistd.h b/include/unistd.h
> > index b1a84d7..0112276 100644
> > --- a/include/unistd.h
> > +++ b/include/unistd.h
> > @@ -16,6 +16,12 @@ extern "C" {
> > #define SEEK_CUR 1
> > #define SEEK_END 2
> >
> > +#if defined(_BSD_SOURCE) && !defined(L_SET)
> > +#define L_SET SEEK_SET
> > +#define L_INCR SEEK_CUR
> > +#define L_XTND SEEK_END
>
> Is L_SET defined somewhere else?
Not in the patch; glibc has it in sys/file.h, which I somehow missed.
I'm inclined to say keep the test and I'll send a patch for that, but
take your pick.
For unknown reasons it looks like sys/file.h has no content except that
protected by _GNU_SOURCE | _BSD_SOURCE -- which sounds like an
unnecessary test.
Isaac Dunham
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.
