On Mon, Oct 01, 2007 at 12:30:07AM -0700, Andrew Morton wrote:
> On Mon, 1 Oct 2007 08:44:48 +0200 Ingo Molnar <[EMAIL PROTECTED]> wrote:
>
> >
> > (lkml Cc:-ed - this might be of interest to others too)
> >
> > * Andy Whitcroft <[EMAIL PROTECTED]> wrote:
> >
> > > WARNING: EXPORT_SYMBOL(foo); should immediately follow its
> > > function/variable
> > > #411: FILE: home/apw/git/linux-2.6/kernel/sched.c:408:
> > > +EXPORT_SYMBOL_GPL(cpu_clock);
> >
> > yes, this is a legit warning and i fix it every time i see it. (I cannot
> > fix this one now because mainline does not have an EXPORT_SYMBOL_GPL for
> > cpu_clock(), it's added in -mm? But i cannot find it in mm either. I'll
> > fix it once i find the patch :)
>
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/broken-out/make-rcutorture-rng-use-temporal-entropy.patch
>
> > > WARNING: printk() should include KERN_ facility level
> > > #4838: FILE: home/apw/git/linux-2.6/kernel/sched.c:4835:
> > > + printk("%-13.13s %c", p->comm,
> > >
> > > WARNING: printk() should include KERN_ facility level
> > > #5622: FILE: home/apw/git/linux-2.6/kernel/sched.c:5619:
> > > + printk("\n");
> > >
> > > WARNING: printk() should include KERN_ facility level
> > > #5633: FILE: home/apw/git/linux-2.6/kernel/sched.c:5630:
> > > + printk("\n");
> > >
> > > WARNING: printk() should include KERN_ facility level
> > > #5640: FILE: home/apw/git/linux-2.6/kernel/sched.c:5637:
> > > + printk(" %s", str);
> > >
> > > These are actually only in debug code and so are unimportant, but
> > > technically they are wrong. This check is a very difficult one in the
> > > face of these constructs. But in this case I think it has got the
> > > report right.
> >
> > this is actually a false positive - as the debug code constructs a
> > printk output _without_ \n. So the script should check whether there's
> > any \n in the printk string - if there is none, do not emit a warning.
> > (if you implement that then i think it can remain a warning and does not
> > need to move to CHECK.)
>
> Yeah, it does that sometimes. I don't think it's fixable within the scope
> of checkpatch. It needs to check whether some preceding printk which might
> not even be in the patch has a \n:
>
> printk(KERN_ERR "foo");
> <100 lines of whatever>
> + printk("bar\n");
>
> we're screwed...
Well, I think that we could do something like this :
#define KERN_CONT ""
...
printk(KERN_ERR "foo");
<100 lines of whatever>
printk(KERN_CONT "bar\n");
It would indicate the author's *intent* which is to continue a line which
has already been started. It would both permit us to remove false positives
from automated scripts, and to read the code more easily. And this is not a
big constaint for the author, given that such constructs are quite rare.
Regards,
Willy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
