Proof positive the "signedness of char *" warning is useless.

Rob Landley rob at landley.net
Fri Dec 2 17:29:37 UTC 2005


Alright, I was too lazy to actually edit my spam filter and I've cooled down 
enough to read the rest of your message.

(I still reserve the right to ignore you.)

> > > It might even
> > > be better to write a perl or gawk script to replaced the unsigned with
> > > signed in the cases where warnings are generated,
> >
> > NO.
>
> There are lots of such warnings.  They all need to be looked at and fixed
> manually in the end.

There has been a warning in networking/traceroute.c for as long as I can 
remember.  Unfortunately, vodz maintains that applet and every time I poke 
him about that sort of thing, he gets upset.  So it may never get fixed.

The warning is this:
/home/landley/busybox/busybox/networking/traceroute.c:296: warning: `align' 
attribute directive ignored

It is not actually a warning that _means_ anything.  On some platforms, the 
align directive is needed, and on others the default value is already 
correct.  I only wanted to get rid of it because at one time I had us down to 
just two warnings and wanted to make a clean sweep of it.  Now we've got four 
new ones in fsck.c and another in uudecode, and perhaps these are meaningful 
and perhaps they aren't.  I like to get the non-meaningful ones to go away so 
that any real messages aren't buried in the noise.

What I object to is the introduction of a new warning that is over 95% noise.  
A warning that complains about code that's been in use and working for years, 
and which is not actually raising a valid objection.

And this is not a position I'm going to defend to you.  There is such a thing 
as a useless warning:

http://groups.google.com/group/linux.kernel/browse_thread/thread/331f881766aa4f09/a6d10fe2e1613f26
http://groups.google.com/group/linux.kernel/browse_thread/thread/754dc7e962794817/6a6adfaa44939718

Keep in mind that gcc 4 broke the build for bash 2.05b.  You have to patch 
bash in order to get it to compile under gcc 4.  Why?  Because at the end of 
a {block} bash has an #ifdef that conditionally chops out some code, and it 
has a label that gotos right before the #ifdef.  When the #ifdef is false, 
the label jumps to the end of the block, and gcc decided "hey, that's 
something no sane person would ever want to do.  Error!".  Despite the fact 
that it's perfectly valid code.  (No, not a warning, an error.)

The patch I'm applying to make it build, I kid you not, is this:

--- bash-2.05b/lib/malloc/malloc.c      2002-06-21 14:16:49.000000000 -0500
+++ new/lib/malloc/malloc.c     2005-10-14 19:30:46.030058320 -0500
@@ -880,6 +880,7 @@

 free_return:

+  do {;} while(0);
 #ifdef MALLOC_STATS
   _mstats.nmalloc[nunits]--;
   _mstats.nfre++;

Yes, LIE TO THE COMPILER, and then it builds...

It is entirely possible for the gcc guys to break stuff and be stupid.  It is, 
in fact, fairly normal in _any_ project that there will be major thinkos.

And a compiler that cries wolf for STUPID REASONS makes it really, really 
tempting to just switch the darn warnings off.

You want to convince me the warnings are valid, show me an actual bug due to a 
SINGLE ONE of the "signedness of char" warnings.  Show me one.

> > Automatic changes to shut the compiler up, where a human didn't look at
> > each and every change being made, are not just churn but DANGEROUS churn.
> > I would _protest_ the application of such a patch.
>
> You misunderstand.  If there are no errors (you claim there are none) with
> the sign mismatches, then fixing them to be not mismatched will introduce
> them?

Yes, it can.

> Doesn't follow, unless the code relies on coding errors to come up 
> with the correct results.

No, I mean that even refactoring can accidentally alter behavior, and over the 
years I've seen what was intended as whitespace changes break stuff.  (This 
is normal, and why you test as you change.)

The code relies on the current behavior to come up with its results.  Who's to 
say what's a coding error?  Is duff's device a coding error?

> Possible, but not likely.  If this code works 
> with the warnings, surely there can't be too much risk in fixing the
> warnings correctly, to have not sign mismatched parameters?

If by "fix" you mean insert gratuitious typecasts, the reason we're not doing 
that is it bloats the code to fix things that aren't actual problems.

If by "fix" you mean change declarations, this can alter behavior and a human 
better have looked at it.

I could get away with defining char to be unsigned because it varies from 
platform to platform and in theory we've already tested it both ways.  (I 
expect there will probably be a big or two anyway.)

> Can there?  
> Yes, I know your answer, but if it is a risk, then I submit its a greater
> risk to have the sign mismatches present in the first place. 

What risk?  Show me a bug we haven't found.  That would be something useful.

> > Because obviously none of these are bugs that have shown up in testing so
> > far, so fresh testing will obviously be equally effective in giving us a
> > false sense of security.
>
> Can't blame programmers not honoring the prototype on either the compiler
> or the test suite.  Damn it people, honor thy father, mother, and
> prototype.

So, you come in to a volunteer project and tell us we're doing it wrong 
because you know better and your way is so perfect.

You forgot to do it in l33t sp33k.

> > Something _useful_ to do would be to fill out the test suite so we have
> > serious coverage.  (Not necessarily fun, but I take a whack at it every
> > couple of weeks.  Big job, but relatively easy to make progress on.  The
> > problem is, I generally find esoteric bugs and wander off on tangents
> > fixing them. :)
>
> I'm quite swamped with a bunch of projects at the moment,

Aren't we all?

http://www.landley.net/code/todo.txt

> but soon, soon I 
> tell you, I will be able to contribute more than just words...

My lack of caring achieves dizzying heights, I tell you.

> I'm cleaning 
> up buildroot, among other things, to be at least self-hosting,

I gave up on buildroot and implemented an alternative, which I'm trying to get 
a new release out of this weekend.

http://www.landley.net/code/firmware

It has been self-hosting in the past, and will be again, but right now User 
Mode linux-2.6.15-rc4 seems to have introduced a new conflict with the uClibc 
headers (possibly related to Mazur's headers) and building a static 
allyesconfig busybox is causing the darn compiler to hang (I haven't narrowed 
this down yet, busybox is triggering it, it's probably a gcc 4.0 issue, but 
User Mode Linux and uClibc may also be involved.)

Right now I'm focusing on getting an automatically generated partitioned hard 
drive image that qemu can boot.  (I actually have that part working, but the 
init scripts for the bootable version are in pieces at the moment...)

> and when  
> that's done, I can write some code for busybox.  Keep me in mind, and if
> there's some coding you want to offload later, I'd be happy to help.

Did you see the TODO list that ships in busybox?

Did you see the recent calls for people to help triage the 80+ issues in 
bugs.busybox.net into "must fix for 1.1", "would be nice", and "not a 1.1 
issue"?

> And 
> by the way, I NEVER mismatch signs on prototypes, except when hardware
> bit-mapped structures force me to recast bit-fields, but then I ALWAYS cast
> them, so I know I did it on purpose.

Unnecessary typecasts suck.  Introducing typecasts to make a compiler warning 
go away is seldom a good idea, and thinking of them as a form of 
_documentation_ is just wrong.

> And my bain is the miserable "blah 
> blah breaks strict-aliasing rules..." warning which showed up around the
> gcc-3.3.4 timeframe.  Yes, I could turn it off easily, but I've learned to
> live with it, even though I am quite a huge fan of the silent build (and I
> don't mean with the .SILENT target),

I'm the guy who was pushing for a silent build under gcc 3.3.  You've managed 
to alienate _me_.

Rob
-- 
Steve Ballmer: Innovation!  Inigo Montoya: You keep using that word.
I do not think it means what you think it means.



More information about the busybox mailing list