Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1614204

Re: [PATCH] ath6kl: Add __printf verification to ath6kl_dbg

From Steve deRosier <derosier@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH] ath6kl: Add __printf verification to ath6kl_dbg
Date 2017-03-31 20:00 +0200
Message-ID <tr906-7Ov-21@gated-at.bofh.it> (permalink)
References <tqRcR-4Gj-5@gated-at.bofh.it> <tr8nn-7xP-9@gated-at.bofh.it> <tr8x3-7BG-9@gated-at.bofh.it> <tr8GJ-7Fk-5@gated-at.bofh.it> <tr8Qp-7JU-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, Mar 31, 2017 at 10:45 AM, Joe Perches <joe@perches.com> wrote:
> 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.
>

Yes. Of course. Lightbulb.

I wasn't even thinking of the fact someone could load the printf
arguments with code that might have side-effects instead of simple
variables to print. I never do it for obvious reasons, but I could see
it happening.

Thanks for spending the time going back and forth with me about it.

Thanks,
- Steve

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH] ath6kl: Add __printf verification to ath6kl_dbg Joe Perches <joe@perches.com> - 2017-03-31 01:00 +0200
  Re: [PATCH] ath6kl: Add __printf verification to ath6kl_dbg Steve deRosier <derosier@gmail.com> - 2017-03-31 19:20 +0200
    Re: [PATCH] ath6kl: Add __printf verification to ath6kl_dbg Joe Perches <joe@perches.com> - 2017-03-31 19:30 +0200
      Re: [PATCH] ath6kl: Add __printf verification to ath6kl_dbg Steve deRosier <derosier@gmail.com> - 2017-03-31 19:40 +0200
        Re: [PATCH] ath6kl: Add __printf verification to ath6kl_dbg Joe Perches <joe@perches.com> - 2017-03-31 19:50 +0200
          Re: [PATCH] ath6kl: Add __printf verification to ath6kl_dbg Steve deRosier <derosier@gmail.com> - 2017-03-31 20:00 +0200

csiph-web