Assorted bug reports with a few fixes for busybox 1.19.0

[Tanguy Pruvot]

The freebsd commit breaks bionic build :

https://github.com/tpruvot/android_external_busybox/commit/1285437217a60194186b2c66a0422205b2dedd08

#ifdef HAVE_MNTENT_H

fixed by moving ifdef after include libbb.h
  

> Am 28.08.2011 05:41, schrieb Denys Vlasenko:
>> On Friday 26 August 2011 03:34, Matthias Andree wrote:
>>> - in kconfig, note that #define _XOPEN_SOURCE 700 prevents the
>>>   definition of SIGWINCH on FreeBSD, as that's a BSD-specific signal.
>>>   I don't know what the best fix is, thus no patch.
>> 
>> Neither do I.

> What parts of the same .c file need _XOPEN_SOURCE 700 to expose symbols,
> or flip behaviour to be standards-compliant?

> Perhaps they should be split from the system-related parts by moving
> them into two distinct .c files.

>>> - several files lack inclusion of libgen.h, causing complaints like
>>> these for dirname. All files that call dirname must #include <libgen.h>.
>> 
>>> coreutils/rmdir.c: In function 'rmdir_main':
>>> coreutils/rmdir.c:72: warning: implicit declaration of function 'dirname'
>>> coreutils/rmdir.c:72: warning: assignment makes pointer from integer
>>> without a cast
>> 
>> Well, "man dirname" says:
>> 
>>        With glibc, one gets the POSIX version of basename() when <libgen.h> is
>>        included, and the GNU version otherwise.
>> 
>> which means that include <libgen.h> is not such a simple matter.

> It is very simple, #include <libgen.h>.

> The application must be prepared for basename()/dirname() modifying
> their argument anyways because you need to support systems where
> basename() trashes your data and you don't have the GNU version.

>> libbb.h says:
>> 
>> /* This is declared here rather than #including <libgen.h> in order to avoid
>>  * confusing the two versions of basename.  See the dirname/basename man page
>>  * for details. */
>> #if !defined __FreeBSD__
>> char *dirname(char *path);
>> #endif
>> 
>> Should we simply remove "#if !defined __FreeBSD_"?

> No, because FreeBSD uses a different prototype and you do not want to
> deprive the compiler of related optimization possibilities:

> FreeBSD's dirname() doesn't tamper with its argument string and declares
> a const char * restricted argument.

> Bottom line: you should #include <libgen.h>.

>>> libbb/speed_table.c:46: warning: large integer implicitly truncated to
>>> unsigned type
>>> libbb/speed_table.c:49: warning: large integer implicitly truncated to
>>> unsigned type
>>> libbb/speed_table.c:52: warning: large integer implicitly truncated to
>>> unsigned type
>>> libbb/speed_table.c:55: warning: large integer implicitly truncated to
>>> unsigned type
>> 
>> Does this go away if you add (unsinged short) cast before
>> Bnnnnnn constants?

> You need 32-bit ints for those anyways as they don't fit into 16-bit
> unsigned short, quoting termios.h:

> /*
>  * Standard speeds
>  */
> #define B0      0
> #define B50     50
> #define B75     75
> #define B110    110
> #define B134    134
> #define B150    150
> #define B200    200
> #define B300    300
> #define B600    600
> #define B1200   1200
> #define B1800   1800
> #define B2400   2400
> #define B4800   4800
> #define B9600   9600
> #define B19200  19200
> #define B38400  38400
> #ifndef _POSIX_SOURCE
> #define B7200   7200
> #define B14400  14400
> #define B28800  28800
> #define B57600  57600
> #define B76800  76800
> #define       B115200 115200
> #define       B230400 230400
> #define       B460800 460800
> #define       B921600 921600
> #define EXTA    19200
> #define EXTB    38400
> #endif  /* !_POSIX_SOURCE */

> And you don't know which system handles that how.

>>> - note that find.c cannot be compiled with clang:
>>>
>>> gmake -f scripts/Makefile.build obj=findutils
>>>   clang -Wp,-MD,findutils/.find.o.d   -std=gnu99 -Iinclude -Ilibbb
>>> -include include/autoconf.h -D_GNU_SOURCE -DNDEBUG -D_LARGEFILE_SOURCE
>>> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
>>> -D"BB_VER=KBUILD_STR(1.19.0)" -DBB_BT=AUTOCONF_TIMESTAMP -O2 -pipe
>>> -fno-strict-aliasing -Wall -Wshadow -Wwrite-strings -Wundef
>>> -Wstrict-prototypes -Wunused -Wunused-parameter -Wunused-function
>>> -Wunused-value -Wmissing-prototypes -Wmissing-declarations
>>> -Wdeclaration-after-statement -Wold-style-definition -fno-builtin-strlen
>>> -finline-limit=0 -fomit-frame-pointer -ffunction-sections
>>> -fdata-sections -fno-guess-branch-probability -funsigned-char
>>> -static-libgcc -falign-functions=1 -falign-jumps=1 -falign-labels=1
>>> -falign-loops=1 -Os     -D"KBUILD_STR(s)=#s"
>>> -D"KBUILD_BASENAME=KBUILD_STR(find)"
>>> -D"KBUILD_MODNAME=KBUILD_STR(find)" -c -o findutils/find.o findutils/find.c
>>> ...
>>> findutils/find.c:896:2: error: illegal storage class on function
>>>         auto action* alloc_action(int sizeof_struct, action_fp f);
>>>         ^
>>> findutils/find.c:897:54: error: expected ';' at end of declaration
>>>         action* alloc_action(int sizeof_struct, action_fp f)
>>>                                                             ^
>>>                                                             ;
>> 
>> Bug in clang. auto is a valid storate class.

> No, it's invalid code:

> - C does not support nested functions.

> - Valid *storage classes for functions* are extern (the default) and
>   static.

> Try compiling with gcc -pedantic -std=c99 (or c89) too see how portable
> those hacks are.

> Oh, and consider C++ - it has nice concepts (like functors) that can be
> way more efficient than any C hacks can be (including space efficiency).

>>> Can this be fixed for the next release?
>> 
>> So far all freebsd related fixes are in this:
>> 
>> http://busybox.net/downloads/fixes-1.19.0/busybox-1.19.0-freebsd.patch

> Two more attached, however, there is no need to collect further freebsd
> patches in busybox patches. I will keep them locally until 1.19.1 or 1.20.0.

> The patch for platform.h is FreeBSD-specific but integrates in a way
> that it's safe for upstream inclusion (so as to keep divergence low).

> The speedtable fix is generic. If you absolutely need to squeeze those
> few bytes, add compile-time checks that (a) all values are actually
> distinct after truncation and only then use unsigned short, else
> unsigned int, and (b) values are small enough.