[KJ] updated kj-devel.pl

[KJ] updated kj-devel.pl

[Vicente Feito]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

According to the kj-devel.pl modified script I've sent (which I guess not much 
people must be using).
I've removed the strlen and sprintf warning cause it's ok to use them, 
amazingly the DO and DONT's document in kerneljanitors.org says that it's not 
correct, but checking the linux/Documentation/cli_sti_removal.txt file I've 
found that it's ok to use it, I've tried contacting the author of the 
document with no luck yet.
Thank you.
PS: I'm not sending patches because this is not the official devel.pl that 
it's on the janitors page. I would like to hear if someone is using it, or 
everybody is using the other tools.

Regards.
Vicente.

[-- Attachment #2: kj-devel.pl --]
[-- Type: application/x-perl, Size: 7611 bytes --]

[-- Attachment #3: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] updated kj-devel.pl

[Randy.Dunlap]

Vicente Feito wrote:
> According to the kj-devel.pl modified script I've sent (which I guess not much 
> people must be using).
> I've removed the strlen and sprintf warning cause it's ok to use them, 
> amazingly the DO and DONT's document in kerneljanitors.org says that it's not 
> correct, but checking the linux/Documentation/cli_sti_removal.txt file I've 
> found that it's ok to use it, I've tried contacting the author of the 
> document with no luck yet.
> Thank you.
> PS: I'm not sending patches because this is not the official devel.pl that 
> it's on the janitors page. I would like to hear if someone is using it, or 
> everybody is using the other tools.

I find the <module>: "You must use the init mechanism" checking
a little too strong.  Not everything in linux/*.c has to be built
as a module, so the tool needs to allow for that.
And not everything is a device driver, but we should be able to
use the tool anyway, to test for all applicable problems... IMO.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This message could be a little better:
Using foo[] it's recommended against *foo

How about:
Using foo[] is recommended over *foo: saves memory references & code.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

spinlock misuse detection is often not straightforward.
E.g., in kernel/acct.c (2.6.11-rc3), I'm getting:
Lock closed on line 115
Lock closed on line 162
Obtained spinlock on line 198, but never unlocked.
acct.c:198:             spin_lock(&acct_globals.lock);


Lock closed on line 247
Lock closed on line 263
Lock closed on line 521
^^^^^^^^^^^^^^ these line numbers appear to be off by 1 line.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

More later.

-- 
~Randy
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] updated kj-devel.pl

[Dave Jones]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

On Mon, Feb 07, 2005 at 04:46:58PM -0800, Randy.Dunlap wrote:

 > spinlock misuse detection is often not straightforward.
 > E.g., in kernel/acct.c (2.6.11-rc3), I'm getting:
 > Lock closed on line 115
 > Lock closed on line 162
 > Obtained spinlock on line 198, but never unlocked.
 > acct.c:198:             spin_lock(&acct_globals.lock);
 > 
 > 
 > Lock closed on line 247
 > Lock closed on line 263
 > Lock closed on line 521
 > ^^^^^^^^^^^^^^ these line numbers appear to be off by 1 line.
 > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

You're probably best off throwing away the spinlock detection code.
Its horrible, and breaks really easily.  I'm almost
embarressed to admit to writing it.

Sparse should be able to do a much better job.

		Dave



[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] updated kj-devel.pl

[Vicente Feito]

Hello.
On Tuesday 08 February 2005 12:46 am, you wrote:
> Vicente Feito wrote:
> > According to the kj-devel.pl modified script I've sent (which I guess not
> > much people must be using).
> > I've removed the strlen and sprintf warning cause it's ok to use them,
> > amazingly the DO and DONT's document in kerneljanitors.org says that it's
> > not correct, but checking the linux/Documentation/cli_sti_removal.txt
> > file I've found that it's ok to use it, I've tried contacting the author
> > of the document with no luck yet.
> > Thank you.
> > PS: I'm not sending patches because this is not the official devel.pl
> > that it's on the janitors page. I would like to hear if someone is using
> > it, or everybody is using the other tools.
>
> I find the <module>: "You must use the init mechanism" checking
> a little too strong.  Not everything in linux/*.c has to be built
> as a module, so the tool needs to allow for that.
> And not everything is a device driver, but we should be able to
> use the tool anyway, to test for all applicable problems... IMO.

You're right, but I think it's important people know that, what do you think 
if I change that by this: "If you're building a module, be concious you must 
use the init mechanism", would that be better? I agree that may be a little 
bit annoying if you're not building anything as module and that message 
displays on you 50 times. What do you think?

> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This message could be a little better:
> Using foo[] it's recommended against *foo
>
> How about:
> Using foo[] is recommended over *foo: saves memory references & code.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Definitively, I'll change that right away, I didn't want to use long messages, 
but in some cases it's better.

>
> spinlock misuse detection is often not straightforward.
> E.g., in kernel/acct.c (2.6.11-rc3), I'm getting:
> Lock closed on line 115
> Lock closed on line 162
> Obtained spinlock on line 198, but never unlocked.
> acct.c:198:             spin_lock(&acct_globals.lock);
>
>
> Lock closed on line 247
> Lock closed on line 263
> Lock closed on line 521
> ^^^^^^^^^^^^^^ these line numbers appear to be off by 1 line.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> More later.
Yes, besides the fact that line numbers are off by 1 in this case, there's a 
big problem with line numbers, because everything is going to the same array, 
so if you use lots of files the line part doesn't give you correct 
information anymore, I'll be fixing that right away, cause I think solving 
that not only permit having correct line numbers but the possibility of 
adding all the .h files too (of course the ones inside the *.c file).

Vicente.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] updated kj-devel.pl

[Vicente Feito]

Hello.
On Tuesday 08 February 2005 01:02 am, you wrote:
> On Mon, Feb 07, 2005 at 04:46:58PM -0800, Randy.Dunlap wrote:
>  > spinlock misuse detection is often not straightforward.
>  > E.g., in kernel/acct.c (2.6.11-rc3), I'm getting:
>  > Lock closed on line 115
>  > Lock closed on line 162
>  > Obtained spinlock on line 198, but never unlocked.
>  > acct.c:198:             spin_lock(&acct_globals.lock);
>  >
>  >
>  > Lock closed on line 247
>  > Lock closed on line 263
>  > Lock closed on line 521
>  > ^^^^^^^^^^^^^^ these line numbers appear to be off by 1 line.
>  > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> You're probably best off throwing away the spinlock detection code.
> Its horrible, and breaks really easily.  I'm almost
> embarressed to admit to writing it.

I think that it could be fixed, or rewritting it shouldn't take that much 
time, so there's no problem about it, if you have other things that you 
wanted to add to this and didn't cause of time or the fact that you though it 
wasn't absolutely necessary or was complicated/time consuming, you can send 
those to me and I'll add it, or try at least.

>
> Sparse should be able to do a much better job.
>
>   Dave

Vicente.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors