svn commit: trunk/busybox/util-linux
Rob Landley
rob at landley.net
Wed Sep 20 17:25:53 UTC 2006
On Wednesday 20 September 2006 10:05 am, Denis Vlasenko wrote:
> > > What will happen if somebody, somewhere in the jungle of included
> > > header files, already used "linenumber" as a name for something?
> >
> > In an applet? The new #define stomps the earlier one for any lines
occurring
> > later in the program.
>
> No, not in applet. "linenumber" can be declared in a library header
> (libc or any other external library a programs compiles against)
> as a global variable. Just like FILE *stdout.
You're replacing a #declare with an enum, at the global level, in an applet.
I see no reason to do this.
> Yes, it's debuggable. But it should be trivially debuggable,
> the same way when you accidentally declare
>
> int syslog; // collides with function declaration
#define syslog 0
int syslog;
The compiler will let you know you screwed up there.
Also, there's a reason that you use ALL_CAPS when you #define stuff. This is
to prevent these sort of collisions. It's a good thing, and the ALL_CAPS
lets you know it's a #define rather than a variable. Your enum looks just
like a variable, despite being more like a #define.
Also, I'm used to python. I'm used to finding things at runtime via a
regression test harness. "It compiled" gives me absolutely zero warm
fuzzies, because there's no guarantee it'll work. The syntax checker is just
the first line of defense, and relying on the syntax checker more strikes me
as a dubious activity at best.
> > > If putchar will expand to ....; something = linenuber;....,
> > > you get NO ERROR. You get nasty bug.
> >
> > You get a behavior change, and you track it down. I've tracked down much
> > worse problems than this.
>
> What if it is on error path? What if it triggers once in a year?
Regression test harness. Test the error path in testsuite.
> > > In other words: C scope rules and typechecking are a Good Thing.
> > > We should use them, because they are designed to save us from
> > > common mistakes.
> >
> > Are you saying that the above putchar() macro would still work when you
made
> > linenumber an enum, or are you just saying it would be easier to track
down
> > the bug?
>
> With enum it will fail to compile right away at enum site.
With the ALL_CAPS name of defines, you don't have collisions in the first
place, _and_ it's clear that you're using a constant rather than a variable.
> > (If linenumber is an enum and you declared it at a global level,
> > and your bog is "something = linenumber" which is _in_ your current scope,
> > how exactly is the enum an improvement?
>
> putchar() is a macro, so "something = linenumber" is in new
> #define's scope if I use putchar() below #define line.
> So gcc happily converts it into "something = 0"
Where did "linenumber" come from in the first place? In the macro you're
trying to use. If you declare it as a local variable, and you've #defined it
to 0, it throws an error. If it's one of the macro arguments, then that
takes precedence in the macro scope over external redefinitions.
#include <stdio.h>
#define thingy(one,two) printf(one,two);
#define two walrus
int main(int argc, char *argv)
{
thingy("hello %s\n","world");
}
gcc hello.c
./a.out
hello world
Can you come up with a real example of this causing an actual problem? Give
me something I can compile, like the above. (P.S. Use ALL_CAPS for your
#define constants, as we do in the real world.)
> > And where did linenumber get declared in putchar, anyway?
> > Wouldn't "int 0;" cause the compiler to throw up?
>
> It was a global variable declared and parsed somewhere up there
> in #include <something.h> from libc, way before gcc had a chance
> to see "#define linenumber 0"
Which is why your #define should have been ALL_CAPS.
> I see that a lot of you folks are really attached to #defines,
Yes, inlcuding me!
> so I wouldn't replace them by enums en masse. However I still think
> that they are unsafe. I will use enums in new code if you
> don't mind, ok?
I'm kind of attached to mount.c. I rewrote the sucker three times. I suspect
your new changes have broken it. (Did you run the regression test script?
Which isn't close to complete, but it's a start.)
If you just want me to keep my hands off of mount.c in perpetuity and forward
all maintenance requests to it to you, perhaps I can do that. Except that
when I did this:
svn update 16000
make distclean
make allnoconfig
make menuconfig: switch on mount, mtab support, an loopback. (Leave nfs off.)
make baseline
svn update
make oldconfig # for NFS_CIFS, which is switched off by default.
make
I got:
function old new delta
bb_getopt_ulflags - 884 +884
append_mount_options 55 183 +128
bb_verror_msg 56 130 +74
mount_it_now 277 330 +53
llist_add_to - 28 +28
bb_perror_msg_and_die 29 47 +18
bb_error_msg_and_die 42 50 +8
msg_eol - 4 +4
logmode - 4 +4
die_sleep - 4 +4
bb_opt_complementally - 4 +4
bb_applet_long_options - 4 +4
xstrdup 33 32 -1
xrealloc 38 37 -1
xmalloc 33 32 -1
busybox_main 196 191 -5
singlemount 633 626 -7
bb_path_mtab_file 10 - -10
bb_error_msg 35 24 -11
mount_main 970 931 -39
bb_vperror_msg 74 30 -44
.rodata 1104 1053 -51
------------------------------------------------------------------------------
(add/remove: 7/1 grow/shrink: 5/9 up/down: 1213/-170) Total: 1043 bytes
This does not fill me with warm fuzzies.
> And Rob, regarding that ssh problem - it's something on ISP
> side, not on busybox.net... I will find out what's going on.
Ouch.
Rob
--
Never bet against the cheap plastic solution.
More information about the busybox
mailing list