Skip to contentSkip to conversation

updated kj-devel.pl

KJ Vicente Feito 3 participants 5 messages Feb 4, 2005 – Feb 8, 2005 Kernel janitors

lore
Share & tools
lore
#0 lore raw

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.

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw
Quoted 11 lines · Vicente Feito

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.

Signature · 5 lines
~Randy
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
#2 lore raw
In reply to Vicente Feito
Quoted 8 lines · Randy.Dunlap

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);

Quoted 5 lines

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

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#3 lore raw
In reply to Vicente Feito

Hello.

Quoted 13 lines · you
Quoted 11 lines · Vicente Feito

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.

Quoted 5 lines

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?

Quoted 8 lines

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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.

Quoted 9 lines

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);

Quoted 7 lines

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
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#4 lore raw
In reply to Vicente Feito

Hello.

Quoted 15 lines · you
Quoted 8 lines · Randy.Dunlap

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);

Quoted 5 lines

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

Quoted 3 lines

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.

Quoted 4 lines

Sparse should be able to do a much better job.

Dave

Vicente.
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors