[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Why commit 68404f13d4bf482?
From:       Rob Landley <rob () landley ! net>
Date:       2010-10-31 20:02:57
Message-ID: 201010311502.57972.rob () landley ! net
[Download RAW message or body]

Adding -wunused-parameter and then typecasting your way around it in dozens of 
files not only bloats the code with lots of brittle pointless annotations, but 
means you can't use that flag later to find potential optimizations where we 
could try to remove the parameter from functions that don't necessarily need 
it.  (Yeah, now we can grep for the annotation.)  Did it find a single actual 
_bug_ in the code where we weren't using a parameter that we should have?  
Isn't that what a test suite is for?

Adding this extra verbiage also means that if we try to get other compilers, 
like LLVM, to build busybox, we have to go through extra steps to explicitly 
add support to them.  It's a step _away_ from genericizing the code.

Plus it's one more thing to learn when reading the busybox code for the first 
time.  I got into busybox development in the first place because the code was 
much simpler and easier to read than other implementations.  Now, between all 
the MAIN_EXTERNALLY_VISIBLE magic (why on earth are you prototyping a function 
on the line above wher eyou declare it??!??) and the UNUSED_PARAM stuff (which 
apparently exists to make sure that the prototype and the declaration can't e 
collapsed together into a single macro, but _must_ be typed out in full both 
times and certified by a public notary)...

I don't understand the motivation for this change.  How was it an improvement?

Rob
-- 
GPLv3: as worthy a successor as The Phantom Menace, as timely as Duke Nukem 
Forever, and as welcome as New Coke.
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic