[Toybox] _GNU_SOURCE definition problem

[Rob Landley]

On 03/07/2012 09:44 AM, Georgi Chorbadzhiyski wrote:
> I'm compiling toybox'es allyesconfig with musl libc and the compilation
> do not succeed because of this.
> 
> Compile toybox...
> toys/id.c: In function ‘id_main’:
> toys/id.c:136:2: warning: implicit declaration of function ‘getgrouplist’ [-Wimplicit-function-declaration]
> toys/id.c:63:8: warning: variable ‘gid’ set but not used [-Wunused-but-set-variable]
> toys/ls.c: In function ‘do_ls’:
> toys/ls.c:170:17: warning: implicit declaration of function ‘major’ [-Wimplicit-function-declaration]
> toys/ls.c:170:17: warning: implicit declaration of function ‘minor’ [-Wimplicit-function-declaration]
> toys/mke2fs.c: In function ‘mke2fs_main’:
> toys/mke2fs.c:444:35: warning: variable ‘dtiblk’ set but not used [-Wunused-but-set-variable]
> toys/netcat.c: In function ‘netcat_main’:
> toys/netcat.c:125:4: warning: passing argument 2 of ‘bind’ from incompatible pointer type [enabled by default]
> /usr/local/musl/include/sys/socket.h:236:5: note: expected ‘const struct sockaddr *’ but argument is of type ‘struct sockaddr_in *’
> toys/netcat.c:146:5: warning: passing argument 2 of ‘getsockname’ from incompatible pointer type [enabled by default]
> /usr/local/musl/include/sys/socket.h:241:5: note: expected ‘struct sockaddr *’ but argument is of type ‘struct sockaddr_in *’

The compiler's being a bit fastidious in netcat there.  Oh well, I
suppose shutting it up with a typecast isn't actually _incorrect_...

> /tmp/ccJRWNtS.o: In function `do_ls':
> ls.c:(.text.do_ls+0x3e2): undefined reference to `minor'
> ls.c:(.text.do_ls+0x3f7): undefined reference to `major'
> collect2: ld returned 1 exit status
> make: *** [toybox] Error 1
> 
> In order for these functions to exist _GNU_SOURCE must be defined.

Actually a lot of stuff is defined by posix-2008 which the linux man
pages say _GNU_SOURCE for.  In reality, _GNU_SOURCE is just the big
"#define every feature we can possibly support simultaneously on"
hammer, and the actual _guard_ is set automatically for you via header
code that defaults to switching on a reasonable standard level.

Why a current glibc wouldn't default to c99 (a 13 year old standard), I
have no idea.  I can see explicitly #defining the "hey, we use posix
2008, deal with it" macro at the start of toys.h, that would be:

  #define _POSIX_C_SOURCE 200809L

The thing is, right now /usr/include/features.h is doing things like this:

  /* If nothing (other than _GNU_SOURCE) is defined,
     define _BSD_SOURCE and _SVID_SOURCE.  */
  #if (!defined __STRICT_ANSI__ && !defined _ISOC99_SOURCE && \
       !defined _POSIX_SOURCE && !defined _POSIX_C_SOURCE && \
       !defined _XOPEN_SOURCE && !defined _XOPEN_SOURCE_EXTENDED && \
       !defined _BSD_SOURCE && !defined _SVID_SOURCE)
  # define _BSD_SOURCE    1
  # define _SVID_SOURCE   1
  #endif

Which means if you #define _POSIX_C_SOURCE you can wind up switching
_off_ other things that get implicitly defined otherwise.

I want to fix this, but we are very much _not_ gnu source.  The FSF has
no claim over this project, and I'm happy to build with a non-gnu
toolchain and libc.

Let's see, change the _GNU_DAMMIT in lib/portability.h to:

#define _FILE_OFFSET_BITS 64
#define _POSIX_C_SOURCE 200809L
#define _XOPEN_SOURCE 600
#define _BSD_SOURCE
#define _SVID_SOURCE

And now the only thing it's complaining about is strndupa(), which
actually _is_ a gnu extension it seems.  Easy short-term fix: mdev.c can
default n for now.

> It is defined in lib/portability.h but since it is included in
> toys.h and toys.h is included after system headers, the definitions
> do not exist.

It shouldn't be after. It shouldn't be separate: toys.h should be
_doing_ the inclusion of most system headers.

Part of the idea here is if you're using something like ccache, toys.h
turns into a single giant preprocessed blob, and compilation goes
faster.  Also it means we're not repeating the same #includes in a
gazillion files and then tweaking "no, on this variant it's string.h not
strings.h" and having to patch it in 12 places.

I've let a few headers go if they're not general purpose, and shoved a
couple others into special purpose files like lib/xregcomp.* and
lib/getmountlist.c where I've previously hit build environments that
didn't have support for that, and thus want them to be able to be
configured out (at the cost of dropping some commands).

But in general, a command should be able to just #include "toys.h" and
no other headers.

> Two possible fixes:
>  1. Define _GNU_SOURCE above system headers in id.c and ls.c
>  2. Move #include "toys.h" above system headers.

3. suck inclusion of other headers into the giant block of headers
toys.h already #includes.

Let's see, grep the #includes in toys/* and filter out toys.h...

All three of the headers included by id.c are already #included by
toys.h, I have no idea why it's separately #including them.  Same with
ls, and the 64 bit file offset thing should probably be in
portability.h.  (A 32 bit offset is actually _2_ gigs since it's signed,
hard drives ahve been larger than that for 20 years. I have a USB
keychain twice that size on my car key, and it's so old all the
writing's worn off...)  In dirname.c, libgen.h is posix-2008 so that
should be in toys.h.  In sort.c math.h is standard...

Ok, what _is_ justifiably separate?

dmesg: sys/klog.h is linux-specific.
insmod.c, rmmod.c: sys/syscall.h is not in posix-2008.
oneit.c: sys/reboot.h is not in posix-2008.

I think that's about it, actually...

> Both are ugly, help!

This needs cleanup.  Thanks for bringing it to my attention.

How do I get a musl test environment, by the way?

> Anyway, with 1 from above and the two patches that I've already posted
> I can successfully build allyesconfig with musl 0.8.6 on 32 bit x86
> machine.

Woot!  I still haven't made it through your macosx cleanups yet. :)

(Working on it...)

Rob