On 24 September 2014 09:45, Ingo Molnar <mi...@kernel.org> wrote: > * Mathias Krause <mini...@googlemail.com> wrote: >> On 21 September 2014 21:49, Arjan van de Ven <ar...@linux.intel.com> wrote: >> > On 9/21/2014 8:26 AM, Mathias Krause wrote: >> >> >> >> - if (pr & _PAGE_PCD) >> >> - pt_dump_cont_printf(m, dmsg, "PCD "); >> >> - else >> >> - pt_dump_cont_printf(m, dmsg, " "); >> >> + pt_dump_cont(m, dmsg, "%-4s", pr & _PAGE_USER ? "USR" : >> >> ""); >> > >> > >> > while you have some nice cleanups in your patch, I can't say I consider >> > this >> > an improvement. >> > Yes the C standard allows ? to be used like this >> > but no, I don't think it improves readability in general. >> >> Not in general, but in this case, it does, IMHO. >> >> > (I think for me the main exception is NULL pointer cases, but this is not >> > one of these) >> >> Apparently such a pattern (using the question mark operator combined >> with a bit test to choose string constants) is used quite often in the >> linux kernel as a simple grep tells me (probably catches a few false >> positives but still a representative number): > > Both can be used (although I too find the original version easier > to read), and it's usually the taste/opinion of the original > author whose choice we prefer.
So I should start writing more code from the scratch than changing others... ;) But concerning this patch, are you interested in the following other pieces: - changing the macros from being a compound statement expression to the common 'do .. while(0)' pattern - use of pr_info()/pr_cont() instead of printk(KERN_INFO/KERN_CONT) - use of format strings in pt_dump_cont_printf() calls - removing the trailing blank before '\n' in the "... # entries skipped ..." message ...or should I just drop patch 2 altogether? Thanks, Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
