mdev

[Denys Vlasenko]

On Wednesday 15 April 2009 01:02, Rob Landley wrote:
> I started looking at your patch, then I started looking at the mdev.c code it 
> applies to, and...
> 
> #if ENABLE_FEATURE_MDEV_CONF
>         parser_t *parser;
> #endif
> 
> Unused stack variables should be optimized out by any half-competent compiler.

But compiler wouldn't warn me when I _use_ the variable outside of
ENABLE_FEATURE_MDEV_CONF block by mistake. With the above ugly #if,
I get compiler error in this case.

> #if !ENABLE_FEATURE_MDEV_CONF
>         mode = 0660;
> #else
> 
> What's wrong with
> 
>   if (!ENABLE_FEATURE_MDEV_CONF) mode = 0660;
> 
> Which becomes
> 
>   if (0) mode = 0660;
> 
> And gets optimized out without an #ifdef.

> # if ENABLE_FEATURE_MDEV_EXEC
>                 char *command = NULL;
> # endif
> 
> Assignments that are never read get optimized away.
> 
> # if ENABLE_FEATURE_MDEV_RENAME
>                 if (!val)
>                         goto line_matches;
>                 aliaslink = val[0];
>                 if (aliaslink == '>' || aliaslink == '=') {
>                         char *a, *s, *st;
> 
> Wrapping the whole thing in if (ENABLE_FEATURE_MDEV_RENAME) { accomplishes the 
> same purpose without having the preprocessor chop up the code...
> 
> The whole file is like this now...

We have different coding style.

I am not against your style, I just am not adopting yours as mine,
just like I do not expect you to adopt my style.

Please send a patch which reduces #ifdef forest
along with make bloatcheck output which shows that
there is no code difference (apart from gcc noise of +/-10 bytes),
and I'll be happy to apply it.

You can even commit it yourself, IIRC you still have write access.
--
vda