On Mon, 2016-08-15 at 13:28 -0700, Kees Cook wrote: > On Mon, Aug 15, 2016 at 12:00 PM, Joe Perches <j...@perches.com> wrote: > > On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: > > > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: > > > > > > #ifndef HAVE_ARCH_BUG > > > #define BUG() do { \ > > > printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, > > > __func__); \ > > > > > > Seems like it should have one? > > > > > > Also, I think we might want to examine WARN() a bit... it doesn't have > > > a log level either, but only a fraction of callers set one: > > > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | > > > wc -l > > > 2735 > > > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc > > > -l > > > 77 > > > > > > If I'm reading checkpatch.pl correctly, it doesn't warn about missing > > > log levels on WARN calls, but I think it should. > > > > > > How do you think is best to clean this up? > > > > > > Mainly, I'd like to add a format string to BUG, or introduce a new > > > BUGish call that takes a format... > > I once suggested something similar awhile ago. > > https://lkml.org/lkml/2008/7/8/261 > > > > I think it's best to remove any KERN_ from the use of > > all the WARN variants and add it to the WARN definitions. > Yeah, that's what I was thinking too. It does mean that the format > needs to be a const char string, though.
No it doesn't mean that. Prefixing KERN_WARNING to the const char[] types and using KERN_WARNING "%pV" could work for the non const char[] types Or maybe use the same KERN_WARNING "%pV" for simpler code. > > Same with BUG. > Yeah, though for full effect, it needs to be plumbed into each > architecture's BUG handler. Mainly what I don't like is that if I do > this on x86: > > pr_err("eek, terrible thing\n"); > BUG(); > > I get a dmesg that read: > > eek, terrible thing > === cut here === > ...traceback, etc > > I'd like the "eek" part to be inside the "cut here". And I'd like BUGs > to be able to be more verbose. Maybe add BUG_MSG/BUG_ON_MSG or some such.