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. 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