[Toybox] _GNU_SOURCE definition problem

Georgi Chorbadzhiyski gf at unixsol.org
Thu Mar 8 04:34:02 PST 2012


Around 03/08/2012 03:05 AM, Rob Landley scribbled:
> 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_...

Should I add the casts, so the compiler would shut up?

>> /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.

strndupa is easily replaced by strndup + free. mdev is not *that* performance
critical :)

>> 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/

dirname.c:#include <libgen.h>
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>
uname.c:#include <sys/utsname.h>
unshare.c:#include <sched.h>
who.c:#include <utmpx.h>

They look reasonable.

> 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

>> 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.

-- 
Georgi Chorbadzhiyski
http://georgi.unixsol.org/
-------------- next part --------------
From 688b2848e252b099df3d03d8ab923afd4efb8d5c Mon Sep 17 00:00:00 2001
From: Georgi Chorbadzhiyski <gf at unixsol.org>
Date: Thu, 8 Mar 2012 14:26:35 +0200
Subject: [PATCH] Cleanup includes in toys/ directory.

---
 toys/id.c      |    3 ---
 toys/ls.c      |    5 -----
 toys/sha1sum.c |    2 +-
 toys/who.c     |    2 +-
 4 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/toys/id.c b/toys/id.c
index 6ae3efe..4503dd7 100644
--- a/toys/id.c
+++ b/toys/id.c
@@ -25,9 +25,6 @@ config ID
 	  -u    Show only the effective user ID
 */
 
-#include <sys/types.h>
-#include <pwd.h>
-#include <grp.h>
 #include "toys.h"
 
 #define FLAG_n (1<<4)
diff --git a/toys/ls.c b/toys/ls.c
index 7610920..6dd64c5 100644
--- a/toys/ls.c
+++ b/toys/ls.c
@@ -25,11 +25,6 @@ config LS
 /* So that we can do 64-bit stat etc... */
 #define _FILE_OFFSET_BITS 64
 
-#include <unistd.h>
-#include <sys/types.h>
-#include <grp.h>
-#include <pwd.h>
-
 #include "toys.h"
 
 #define FLAG_a 1
diff --git a/toys/sha1sum.c b/toys/sha1sum.c
index 3229cd1..6947772 100644
--- a/toys/sha1sum.c
+++ b/toys/sha1sum.c
@@ -20,7 +20,7 @@ config SHA1SUM
 	  Calculate sha1 hash of files (or stdin).
 */
 
-#include <toys.h>
+#include "toys.h"
 
 struct sha1 {
 	uint32_t state[5];
diff --git a/toys/who.c b/toys/who.c
index f6559bc..75c821d 100644
--- a/toys/who.c
+++ b/toys/who.c
@@ -21,7 +21,7 @@ config WHO
 */
 
 #include "toys.h"
-#include <time.h>
+
 #include <utmpx.h>
 
 void who_main(void)
-- 
1.7.5.1



More information about the Toybox mailing list