[Toybox] _GNU_SOURCE definition problem

[Rob Landley]

On 03/08/2012 06:34 AM, Georgi Chorbadzhiyski wrote:
> Around 03/08/2012 03:05 AM, Rob Landley scribbled:
>> On 03/07/2012 09:44 AM, Georgi Chorbadzhiyski wrote:
>>> 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_...
> 
> Should I add the casts, so the compiler would shut up?

I already did: http://landley.net/hg/toybox/rev/532

>> 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.
> 
> strndupa is easily replaced by strndup + free. mdev is not *that* performance
> critical :)

I see your patch for that in my queue, but probably won't be able to
catch up until the weekend.

>>> 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.
> 
> After the cleanup patch (see bellow) here are the includes in toys/

I did this and checked it in yesterday as well:

  http://landley.net/hg/toybox/rev/533

> dirname.c:#include <libgen.h>

It's posix 2008, I consolidated it.

> dmesg.c:#include <sys/klog.h>
> insmod.c:#include <sys/syscall.h>
> oneit.c:#include <sys/reboot.h>
> rmmod.c:#include <sys/syscall.h>
> sort.c:#include <math.h>

math.h is also posix 2008.

The header list is under "base definitions":

http://pubs.opengroup.org/onlinepubs/9699919799/idx/head.html

> uname.c:#include <sys/utsname.h>
> unshare.c:#include <sched.h>
> who.c:#include <utmpx.h>

All three also posix standard, and merged in yesterday's patch.

>> 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?
> 
> get musl from : http://www.etalabs.net/musl/download.html
> untar, make and make install
> compile toybox with make CC=musl-gcc

Compiler wrapper.  Got it.

>>> 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. :)
> 
> Ok, the attached patch cleans up includes in toys/ directory. With this
> and the other two patches (replace index with strchr and stop using
> strndupa) the compilation with musl is possible.

I applied the cleanups I was talking about to the repository at the same
time I sent the above message.  Did my cleanups (based on posix) do what
you needed?

Rob