linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Niels de Vos <ndevos@redhat.com>
Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] omap2/omapfb: make DBG() more resistant in if-else constructions
Date: Tue, 10 May 2011 09:42:01 +0000	[thread overview]
Message-ID: <BANLkTinEJZ=fvJmWRkQ7kKyxbFRaJnum7g@mail.gmail.com> (raw)
In-Reply-To: <1305019249-9898-1-git-send-email-ndevos@redhat.com>

On Tue, May 10, 2011 at 11:20, Niels de Vos <ndevos@redhat.com> wrote:
> When DBG() is used in a simple if-else, the resulting code path
> currently depends on the definition of DBG(). Inserting the statement in
> a "do { ... } while (0)" prevents this possible misuse.
>
> Signed-off-by: Niels de Vos <ndevos@redhat.com>

> --- a/drivers/video/omap2/omapfb/omapfb.h
> +++ b/drivers/video/omap2/omapfb/omapfb.h
> @@ -34,8 +34,10 @@
>  #ifdef DEBUG
>  extern unsigned int omapfb_debug;
>  #define DBG(format, ...) \
> -       if (omapfb_debug) \
> -               printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__)
> +       do { \
> +               if (omapfb_debug) \
> +                       printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__); \
> +       while (0)

Where's the closing '}'?

>  #else
>  #define DBG(format, ...)

BTW, no printf()-style format checking here.

>  #endif

What about using the standard pr_debug()/dev_dbg() instead?
With dynamic debug, it can be enabled at run time.
As a bonus, you get printf()-style format checking if debugging is disabled.


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

  reply	other threads:[~2011-05-10  9:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-10  9:20 [PATCH] omap2/omapfb: make DBG() more resistant in if-else constructions Niels de Vos
2011-05-10  9:42 ` Geert Uytterhoeven [this message]
2011-05-10 10:57   ` Niels de Vos
2011-05-10 11:07   ` [PATCH V2] " Niels de Vos
2011-05-10 12:16     ` [PATCH V2] omap2/omapfb: make DBG() more resistant in if-else Tomi Valkeinen
2011-05-10 12:08   ` [PATCH] " Tomi Valkeinen
2011-05-10 12:14     ` [PATCH] omap2/omapfb: make DBG() more resistant in if-else constructions Geert Uytterhoeven
2011-05-10  9:49 ` [PATCH] omap2/omapfb: make DBG() more resistant in if-else Premi, Sanjeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='BANLkTinEJZ=fvJmWRkQ7kKyxbFRaJnum7g@mail.gmail.com' \
    --to=geert@linux-m68k.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=ndevos@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).