Re: _BSD_SOURCE support for musl ready

[Rich Felker <dalias@aerifal.cx>]

On Sun, May 20, 2012 at 11:26:51PM -0700, Isaac Dunham wrote:
> I think the _BSD_SOURCE support is ready to merge now.

Great. Quick review; not sure I caught all the issues...

> diff --git a/include/fcntl.h b/include/fcntl.h
> index 63f1beb..b776d38 100644
> --- a/include/fcntl.h
> +++ b/include/fcntl.h
> [...]
> +
> +#ifndef _UNISTD_H
> +#define F_OK 0
> +#define R_OK 4
> [...]

All _XXX_H macros should be considered entirely internal to the header
they're used to protect. Aside from that, this sort of mechanism makes
the logic in unistd.h very ugly (since it would have to check if
fcntl.h was already included with _GNU_SOURCE or _BSD_SOURCE defined).

Since the definitions are exactly the same, multiple #defines are
legal in C. If they work in C++ too, and if the compiler does not
issue ugly warnings, I would just leave it be. Otherwise you could
#undef them all first, or use #ifdef F_OK.

> diff --git a/include/netdb.h b/include/netdb.h
> index 33b7a0a..d1fe9a8 100644
> --- a/include/netdb.h
> +++ b/include/netdb.h
> @@ -5,7 +5,7 @@
> [...]
> +
> +int innetgr(const char *, const char *, const char *, const char *);

This should be a separate patch, and should add stubs for the rest of
the netgr functions at the same time (weak aliases in ent.c). Of
course innetgr needs to be in its own file since it's not a weak alias
and would pollute the standard namespace if put in ent.c.

> diff --git a/include/signal.h b/include/signal.h
> [...]
> -#ifdef _GNU_SOURCE
> +#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
>  typedef void (*sighandler_t)(int);
> +typedef sighandler_t sig_t;
>  void (*bsd_signal(int, void (*)(int)))(int);
> +#endif

I think sig_t should be BSD-only. That's how it is in glibc, and since
it's so illogically named and short, I'd rather keep it out of the
namespace even with _GNU_SOURCE.

In glibc, sighandler_t is GNU-only (not in BSD). So it might make
sense to just have these be separate #ifdefs.

> diff --git a/include/string.h b/include/string.h
> index 8cf0ee9..9715713 100644
> --- a/include/string.h
> +++ b/include/string.h
> [...]
>  
> +#if defined(_BSD_SOURCE) || defined(_GNU_SOURCE)
> +int bcmp (const void *, const void *, size_t);
> +void bcopy (const void *, void *, size_t);
> +void bzero (void *, size_t);
> +int strcasecmp (const char *, const char *);
> +int strncasecmp (const char *, const char *, size_t);
> +char *index (const char *, int);
> +char *rindex (const char *, int);
> +int ffs (int);
> +#endif

Wasn't all of this already coming from strings.h?


That was just from a quick glance, so there might be some other issues
remaining, but overall it looks good. Thanks!

Rich