All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Steve deRosier <derosier@gmail.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ath6kl: Add __printf verification to ath6kl_dbg
Date: Fri, 31 Mar 2017 10:45:07 -0700	[thread overview]
Message-ID: <1490982307.27353.14.camel@perches.com> (raw)
In-Reply-To: <CALLGbRJRstmGL-o2ds4SQ5hDNVBcfP23JnRt5qRrVg-ix-8Hiw@mail.gmail.com>

On Fri, 2017-03-31 at 10:34 -0700, Steve deRosier wrote:
> On Fri, Mar 31, 2017 at 10:23 AM, Joe Perches <joe@perches.com> wrote:
> > On Fri, 2017-03-31 at 10:19 -0700, Steve deRosier wrote:
> > > On Thu, Mar 30, 2017 at 3:57 PM, Joe Perches <joe@perches.com> wrote:
> > > > Fix fallout too.
> > 
> > []
> > > My only question is why bother doing a format check on something
> > > that's going to be compiled out anyway?
> > 
> > To avoid introducing defects when writing new code
> > and not using the debugging code path.
> > 
> 
> Fair enough. And I totally agree with the defensive programming here
> in that case and feel it's worth the tradeoff (if indeed there really
> is any cost, I'm unsure what gcc actually does in this instance).
> 
> For sake of discussion though - shouldn't anything not using the debug
> code path in this case always be of the form that compiles out? ie
> would be empty functions intended here just to make compilation work
> and the code that depends on it simpler? Thus, there really should
> never be a risk of introducing said defects. If any "real" code were
> put in that else clause, that'd be a big red-flag in the review of
> said hypothetical patch.

Generically, all debugging forms should strive to avoid side-effects.

For instance, look at no_printk/pr_debug in the #ifndef DEBUG paths.

It uses if (0) to avoid compilation of arguments that might be
function calls or volatile accesses and so might have side-effects
altogether.

include/linux/printk.h-/*
include/linux/printk.h- * Dummy printk for disabled debugging statements to use whilst maintaining
include/linux/printk.h- * gcc's format checking.
include/linux/printk.h- */
include/linux/printk.h:#define no_printk(fmt, ...)                              \
include/linux/printk.h-({                                                       \
include/linux/printk.h- do {                                            \
include/linux/printk.h-         if (0)                                  \
include/linux/printk.h-                 printk(fmt, ##__VA_ARGS__);     \
include/linux/printk.h- } while (0);                                    \
include/linux/printk.h- 0;                                              \
include/linux/printk.h-})
i

  reply	other threads:[~2017-03-31 17:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-30 22:57 [PATCH] ath6kl: Add __printf verification to ath6kl_dbg Joe Perches
2017-03-31 17:19 ` Steve deRosier
2017-03-31 17:23   ` Joe Perches
2017-03-31 17:34     ` Steve deRosier
2017-03-31 17:45       ` Joe Perches [this message]
2017-03-31 17:54         ` Steve deRosier
2017-04-13 12:43 ` Kalle Valo
2017-04-13 12:43   ` Kalle Valo

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=1490982307.27353.14.camel@perches.com \
    --to=joe@perches.com \
    --cc=derosier@gmail.com \
    --cc=kvalo@qca.qualcomm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.