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

[Steve deRosier]

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