Re: [DRIVER SUBMISSION] DRBD wants to go mainline

[Satyam Sharma]

On 7/22/07, Satyam Sharma <satyam.sharma@xxxxxxxxx> wrote:
> On 7/22/07, Lars Ellenberg <lars.ellenberg@xxxxxxxxxx> wrote:
> [...]
> >  8 ERROR: Macros with multiple statements should be enclosed in a do - while loop
>
> You don't want to break if-else constructs.
>
> >  1 ERROR: Macros with complex values should be enclosed in parenthesis
>
> I'm not sure know when / why checkpatch.pl reports this. I don't want to
> pull your tree so can't check for myself, but note some obvious rules:
>
> 1. Macro definition must always include parenthesis around the entire
> definition (unless it's a do { somehting; } while (0) kind of macro).
> 2. Use parenthesis around wherever it uses the passed argument(s).
>
> >     these are "netlink packet definition language" in drbd_nl.h,
> >     which is sort of clean, and preprocessor magic in drbd_nl.c.
> >     suggestions how to handle this cleanly,
> >     without making it more ugly?
> >     autogenerate code by other means?
> >     write it out by hand, and lose the nice and clean drbd_nl.h?
>
> 1. Open-code it if it is trivial enough and there's no point
> obfuscating anything.
> 2. Consider making the macro a function. Probably inline, most probably not.
> 3. Macros must NOT evaluate the passed argument twice -- consider the
> possible damages of someone who doesn't know it's a macro and therefore
> passing in an expression that has side-effects (*boom, crash*).


Some more macro tips for you:

If the macro contains multiple statements but must also return a value
to the caller (i.e. say it can appear on RHS of an assignment), use the:

#define foo ({ something; lastthing; })

kind of idiom. So the return value of the last statement ("lastthing;"
above) in there (obviously) becomes the return value of the whole
macro. So if you need to define an internal variable inside the macro,
and the return value of the macro should be that internal variable itself,
use the:
#define bar ({ int __mvar; __mvar = something; otherstuff; __mvar; })
kind of idiom.

Of course, if you really try to pull off the above stunts, we'll probably
ask you to go back and make that a proper function in the first place :-)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/