On Thu, 27 Sep 2007, Vegard Nossum wrote:
>   It should be possible to optimize out multi-line (block) entries
> based on log-level filtering even though the log-level is only given
> in the first call (the initializer). It may take the shape of an
> if-block that spans several macros. This is not very elegant or robust
> if the macros are used incorrectly, however. Aborting a message can
> also be hard this way (since the abort would usually appear inside an
> if-statement that tests for some abnormal condition, thus appear in a
> different block, and thoroughly mess up the bracket order).
> 
> Example: {
>       #define kprint_block_init(block, loglevel)              \
>               if(loglevel > CONFIG_KPRINT_LOGLEVEL_MAX) {     \
>                       kprint_real_block_init(block, loglevel);
> 
>       #define kprint_block(block, fmt, ...)                   \
>               kprint_real_block(block, fmt, ## __VA_ARGS__);
> 
>       #define kprint_block_flush(block)                       \
>                       kprint_real_block_flush(block);         \
>               }
> 
>       /* Thus, this C code: */
>       kprint_block_init(&block, KPRINT_INFO);
>       kprint_block(&block, "Hello world");
>       kprint_block_flush(&block);
> 
>       /* Would pre-process into this: */
>       if(6 < 4) {
>               kprint_real_block_init(&block, 6);
>               kprint_real_block(&block, "Hello world");
>               kprint_block_flush(&block);
>       }
> }

If-blocks spanning macros are really dangerous!

E.g. an Ethernet driver may want to do:

        kprint_block(&block, "MAC ");
        for (i = 0; i < 6; i++) {
                card->mac[i] = obtain_mac_byte_from_hw(i);
                kprint_block(&block, "%02x", card->mac[i]);
        }

This looks (and should be) innocent, but the actual MAC addres retrieval
would never be executed if loglevel <= CONFIG_KPRINT_LOGLEVEL_MAX.

Can't you store the loglevel in the kprint_block and check it in all
successive kprint_*() macros? If gcc knows it's constant, it can optimize
the non-wanted code away. As other fields in struct kprint_block cannot be
constant (they store internal state), you have to split it like:

    struct kprint_block {
            int loglevel;
            struct real_kprint_block real;  /* internal state */
    }

and pass &block.real() instead of &block to all successive internal functions.
I haven't tried this, so let's hope gcc is actually smart enough...

Gr{oetje,eeting}s,

                                                Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                                            -- Linus Torvalds
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to