svn commit: trunk/busybox/coreutils

[Rob Landley]

On Saturday 23 September 2006 7:56 am, Bernhard Fischer wrote:
> On Fri, Sep 22, 2006 at 12:11:59PM -0700, landley at busybox.net wrote:
> >Author: landley
> >Date: 2006-09-22 12:11:59 -0700 (Fri, 22 Sep 2006)
> >New Revision: 16192
> >
> >Log:
> >Follow-up to 16172: this also doesn't produce a warning for me on gcc 4.1,
> >without having to feed the compiler nonsense.
> 
> You aren't being defiant, are you?

Thanks ever so much.

> Read the code.

I did.

If there's an actual way for it to be used uninitialized (which I can't see) 
then don't add a comment "to humor gcc".  It's either a real problem, or it 
isn't.

> /tmp/busybox/coreutils/uudecode.c: In function 'uudecode_main':
> /tmp/busybox/coreutils/uudecode.c:144: warning: 'decode_fn_ptr' may be
> used uninitialized in this function

Reading the warning is not reading the code.  The code says:

	char *line_ptr = NULL;

        if (strncmp(line, "begin-base64 ", 13) == 0) {
            line_ptr = line + 13;
            decode_fn_ptr = read_base64;
        } else if (strncmp(line, "begin ", 6) == 0) {
            line_ptr = line + 6;
            decode_fn_ptr = read_stduu;
        }

        if (line_ptr) {

No way to get through that with line_ptr != NULL yet decode_fn_ptr 
uninitialized, and the only users of decode_fn_ptr are in the if.  The 
compiler's warning is _wrong_ here.  It's not following logic that requires 
paying attention to two variables to track.

> What version of gcc-4.1 do you have?

Configured 
with: ../configure --prefix=/opt/timesys/toolchains/i686-linux --mandir=/opt/timesys/toolchains/i686-linux/share/man --infodir=/opt/timesys/toolchains/i686-linux/share/info --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-libgcj-multifile --enable-languages=c,c++ --with-cpu=generic --program-prefix=i686-linux- --build=i686-linux --host=i686-linux --target=i686-linux --disable-libgcj --disable-libgomp --disable-libmudflap --disable-libssp
Thread model: posix
gcc version 4.1.0 20060304 ( 4.1.0-3)

> How do you use gcc-4.1 to compile busybox?

PATH=/opt/timesys/toolchains/i686-linux/bin:$PATH make CONFIG_STATIC=y \ 
CROSS=i686-uclibc-

It's pretty easy to tell when it used the wrong one, because it won't be 
linked against uClibc.

> Reverting that fix in the face of you having added -Werror is not a
> sensible thing to do. YMMV

No, reverting -Werror for the broken gcc 4.1.0 was the sensible thing to do, 
which I did back in July:

http://busybox.net/downloads/patches/svn-15761.patch

> r15761 | landley | 2006-07-31 18:56:17 -0400 (Mon, 31 Jul 2006) | 9 lines
> Changed paths:
>    M /trunk/busybox/Rules.mak
...
> 3) Move the -Werror into the gcc 4.0 on i386 test, because gcc
> 4.1 is broken and produces warnings for things that provably aren't
> incorrect.

The warning generator in that version is _broken_, and changing our code to 
humor a broken warning generator is not something I'm interested in doing.

I can also check to make sure it's not added for gcc before 3.x, but those old 
versions aren't exactly a priority.

I haven't been paying attention to 2.95, since it's obsolete.

I'll take another stab at untangling the logic so the compiler can follow it 
more easily, though.  Possibly feeding it the third "else" with a continue to 
make the big if() go away...

Ok, try it now.

Rob
-- 
Never bet against the cheap plastic solution.