From: Valdis.Kletnieks at vt.edu <hidden> Date: 2014-09-17 03:42:18
On Tue, 16 Sep 2014 20:35:35 -0500, Greg Donald said:
fs/* currently contains 96,375 errors and 22,555 warnings.
[/usr/src/linux-next] find fs -type f -name '*.[ch]' | xargs cat | wc -l
1138557
96K errors seemed to be a tad.... high. So.. doublechecking..
[/usr/src/linux-next] for i in `find fs -type f -name '*.[ch]'`; do scripts/checkpatch.pl -f $i; done > /tmp/fs.check
And sure enough, looking through egrep '^total|^fs' /tmp/fs.check, we find
4 really big offenders.
total: 9823 errors, 0 warnings, 7933 lines checked
fs/nls/nls_cp932.c has style problems, please review.
total: 19512 errors, 0 warnings, 9482 lines checked
fs/nls/nls_cp950.c has style problems, please review.
total: 27672 errors, 0 warnings, 13946 lines checked
fs/nls/nls_cp949.c has style problems, please review.
total: 27252 errors, 4 warnings, 11111 lines checked
fs/nls/nls_cp936.c has style problems, please review.
And git blame says this about nls_cp932.c:
b9ec0339d8e22 (Denys Vlasenko 2007-10-16 23:29:54 -0700 16) static const wchar_t c2u_81[256] = {
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 17) 0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x00-0x07 */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 18) 0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x08-0x0F */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 19) 0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x10-0x17 */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 20) 0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x18-0x1F */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 21) 0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x20-0x27 */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 22) 0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x28-0x2F */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 23) 0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x30-0x37 */
You're looking at 56 checkpatch errors right there.
Yes, it's been missing the 8 spaces after the 8 commas since the initial import
into git almost a decade ago. And those lines are already 74 characters, so you
can't add the missing blanks after the ','s without putting the line over 80
chars... So maybe it's time to actually *think* a bit about what checkpatch
is telling us.
Excluding those 4 files, we're down to 12116 errors which works out
to one every 93 lines.
grep ^ERROR /tmp/fs.check | sort | uniq -c | sort -nr | head
84812 ERROR: space required after that ',' (ctx:VxV)
2105 ERROR: trailing whitespace
1518 ERROR: "foo * bar" should be "foo *bar"
1393 ERROR: code indent should use tabs where possible
989 ERROR: do not use assignment in if condition
Wow. Another 2,105 "errors" are trailing whitespace, and another
1,393 are places somebody used spaces instead of tabs. Oh, the humanity.
Especially since these are invisible to somebody reading the code (unlike
the foo * bar/ foo *bar thing).
Exclude those two cases and we're up to one "error" every 132 lines.
(For comparison, the first few most popular warnings:
6215 WARNING: line over 80 characters
3241 WARNING: quoted string split across lines
2715 WARNING: Missing a blank line after declarations
1771 WARNING: please, no spaces at the start of a line
1742 WARNING: __constant_cpu_to_le32 should be cpu_to_le32
1030 WARNING: space prohibited between function name and open parenthesis '('
681 WARNING: please, no space before tabs
530 WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
Obviously, the checkpatch distinction of warning versus error could itself
use some tuning. Though it's right that somebody should probably
smack fs/cifs/nterr.h around with a large trout, that's a bunch of
precedence bugs waiting to happen.
net/* currently contains 3,366 errors and 19,536 warnings.
[/usr/src/linux-next] find net -type f -name '*.[ch]' | xargs cat | wc -l
850658
That works out to 1 "error" in net/ every 252 lines.
Meanwhile drivers/staging/* contains 19,004 errors and 35,292 warnings
find drivers/staging -type f -name '*.[ch]' | xargs cat | wc -l
1037877
That works out to 1 error every 54 lines. You'll have to fix around 7,800
of those 19,004 errors before the code is as clean as fs/, and 15,000 of
them to get drivers/staging up to net/ standards. Better get patching. :)
(And this sort of analysis is exactly *why* people need to apply their brains
when looking at checkpatch output....)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140916/97bc6e62/attachment.bin
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks at vt.edu wrote:
(And this sort of analysis is exactly *why* people need to apply their brains
when looking at checkpatch output....)
No one has ever said that they shouldn't.
Remember, I know _lots_ of kernel developers who started with just
"checkpatch cleanups on staging drivers" and they moved on to much
"higher" roles in the kernel developer ecosystem (jobs, maintainers of
subsystems, keynote talks at conferences, etc.)
Don't "po po" it as something that shouldn't be a valid place to start,
it is, and is why I do the work to review all of the many thousands of
staging patches every release cycle.
No one is forcing you to write those patches, or read / review them, so
don't discourage others to provide them either please. I most certainly
do not.
thanks,
greg k-h
On Wed, Sep 17, 2014 at 10:26 AM, Greg KH [off-list ref] wrote:
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks at vt.edu wrote:
quoted
(And this sort of analysis is exactly *why* people need to apply their brains
when looking at checkpatch output....)
No one has ever said that they shouldn't.
Remember, I know _lots_ of kernel developers who started with just
"checkpatch cleanups on staging drivers" and they moved on to much
"higher" roles in the kernel developer ecosystem (jobs, maintainers of
subsystems, keynote talks at conferences, etc.)
thats encouraging information . :)
thanks Greg
Don't "po po" it as something that shouldn't be a valid place to start,
it is, and is why I do the work to review all of the many thousands of
staging patches every release cycle.
No one is forcing you to write those patches, or read / review them, so
don't discourage others to provide them either please. I most certainly
do not.
thanks,
greg k-h
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
From: Robert P. J. Day <hidden> Date: 2014-09-17 10:39:12
On Tue, 16 Sep 2014, Greg KH wrote:
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks at vt.edu wrote:
quoted
(And this sort of analysis is exactly *why* people need to apply their brains
when looking at checkpatch output....)
No one has ever said that they shouldn't.
Remember, I know _lots_ of kernel developers who started with just
"checkpatch cleanups on staging drivers" and they moved on to much
"higher" roles in the kernel developer ecosystem (jobs, maintainers
of subsystems, keynote talks at conferences, etc.)
Don't "po po" it as something that shouldn't be a valid place to
start, it is, and is why I do the work to review all of the many
thousands of staging patches every release cycle.
No one is forcing you to write those patches, or read / review them,
so don't discourage others to provide them either please. I most
certainly do not.
as someone who started out this way (submitting "trivial" patches,
and still do from time to time) and who now makes a living teaching
kernel programming and embedded linux and device drivers, perhaps i
can add some perspective, and also explain why nick krause is
monstrously off-base in everything he touches.
of *course* it's useful that beginners get the opportunity to submit
trivial patches based on nothing but perhaps checkpatch warnings --
it's a great way to get your feet wet, burn in the lessons of how to
write and submit a proper patch, and so on and so on. but notice the
really important point gregkh makes here:
Remember, I know _lots_ of kernel developers who started with just
"checkpatch cleanups on staging drivers" and they moved on to much
"higher" roles in the kernel developer ecosystem (jobs, maintainers
of subsystems, keynote talks at conferences, etc.)
the obvious implication is that, while you *start* simple, the goal is
to ***move up***. trivial, style-based patches are a great
*introduction*, but everyone should have the eventual goal of more and
more sophisticated patches and contributions involving tweaking code
and eventually writing new subsystems, etc, etc. and this is where
nick krause is failing miserably.
nick shows absolutely no interest in understanding the code he's
looking at. his approach to patches is to blindly run checkpatch, look
at the first warning, go to that file, and try to "fix" it, without in
any way whatsoever trying to understand the code in a larger context.
if checkpatch says to add blank lines, nick will add blank lines,
after which he will understand no more about the code than when he
started, which is why, regardless of how long nick does this, he will
never, ever, ever understand any more about the kernel than he does
now.
nick has made it obvious he has no interest in actually
understanding how the kernel works -- all he is obsessed with is
getting his name into the git log as the author of a patch; hence, his
relentless labour in submitting variation after variation of a patch
that does nothing more than add three blank lines to a single file.
nick has long since lost sight of what that single source file is
doing (if he ever even cared what it did in the first place). he is
now in a very unhealthy place where he is going to get those blank
lines in there if it kills him or pisses off every single person on
the kernelnewbies mailing list, and that is precisely why working with
him is a complete waste of time.
other beginners will start where nick is now and, in short order,
they will progress to bigger and better things -- as greg kh suggests,
writing code, becoming subsystem maintainers, giving keynotes. nick
will never, ever, ever do any of this; five years from now, nick will
still blindly be running checkpatch, then going to files looking for
blank lines to add. he will never progress beyond that, simply because
he's doing this for all the wrong reasons.
nick doesn't care about how the kernel works. nick just wants to get
a patch in there somewhere ... anywhere, it doesn't matter. which is
why he is not worth anyone's time. nick will never be a useful
contributor to the kernel community because, in the end, he doesn't
really care about the kernel.
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
From: Robert P. J. Day <hidden> Date: 2014-09-17 11:20:42
On Tue, 16 Sep 2014, Greg KH wrote:
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks at vt.edu wrote:
quoted
(And this sort of analysis is exactly *why* people need to apply their brains
when looking at checkpatch output....)
No one has ever said that they shouldn't.
Remember, I know _lots_ of kernel developers who started with just
"checkpatch cleanups on staging drivers" and they moved on to much
"higher" roles in the kernel developer ecosystem (jobs, maintainers
of subsystems, keynote talks at conferences, etc.)
Don't "po po" it as something that shouldn't be a valid place to
start, it is, and is why I do the work to review all of the many
thousands of staging patches every release cycle.
No one is forcing you to write those patches, or read / review them,
so don't discourage others to provide them either please. I most
certainly do not.
so while i'm waxing philosophical, some other thoughts that occurred
to me that reflect on how i got started, and ways to become a more and
more useful contributor to the kernel for newbies (and i'm willing to
be corrected on any of this).
first, stop with the blind running of checkpatch unless you're
willing to take the results and examine the *context* in which they
occur. that file needs blank lines? ok, does it reside in a directory
of related files that could *also* use some blank lines? then do them
*all* -- don't waste peoples' time fixing one file at a time. if you
can do the same stylistic cleanup on an entire subsystem, go for it,
and don't clutter up the git log with numerous trivial commits.
*but* ... don't try to sneak functional changes in there at the same
time. if it's a style cleanup, then it's a *style* cleanup. one thing
at a time. but there are other places you can make a name.
first, read the Documentation/ directory -- there's lots of content
there, and quite a bit of it is out of date or just plain obsolete.
and if you want people to love you, improve the documentation. but,
see, that's going to take some work. and that's because it requires
you to read the documentation, then go off and examine the
corresponding code to see if it still matches. and why is this good?
because while updating the Documentation/ content is safe and can't
break anything, the side effect is that you *learn* about that
particular subsystem, you get some nifty patches into the kernel, and
you make lots and lots of friends.
another place to get cheap patches is to repair any kernel-doc
warnings, and there are *always* plenty of those. again, fixing
kernel-doc content shouldn't break anything, it should be easy
patching, and it normally requires you to at least examine the code to
make sure you're fixing it properly. so, you get patches into the
kernel, and you learn a bit more about some code. win-win.
last point here regarding something gregkh wrote -- yes, it's fine
to *start* with simple stylistic cleanup, especially if checkpatch
does all the work for you. but remember, that's low-hanging fruit, and
you shouldn't be greedy and try and do all of it. if stylistic cleanup
is a way for beginners to get their first patches into the kernel,
then don't be a pig and try to do it all -- leave some for others to
cut their teeth on. and what is the point of all this?
quite simply, this is also why nick krause will never be a useful
member of the kernel community. i suggested a while back that nick
could start with improving the documentation, for all the reasons i
mentioned above. his response was that he didn't know enough to do
that, which is an astonishing thing to admit. if you don't know enough
to improve the basic documentation, you have no right to be mucking
around in the code.
and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output of
checkpatch, which means he wants to grab all the trivial cleanup (the
low-hanging fruit, as it were) for himself, and not leave any for
others. rather than take the time to understand the code, nick wants
checkpatch to do all the work for him. in the end, nick doesn't want
to do any work or understand how the kernel actually works -- he just
wants patches, and he wants them as quickly and cheaply as possible.
anyway, it's time for coffee.
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks at vt.edu wrote:
quoted
(And this sort of analysis is exactly *why* people need to apply their brains
when looking at checkpatch output....)
No one has ever said that they shouldn't.
Remember, I know _lots_ of kernel developers who started with just
"checkpatch cleanups on staging drivers" and they moved on to much
"higher" roles in the kernel developer ecosystem (jobs, maintainers
of subsystems, keynote talks at conferences, etc.)
Don't "po po" it as something that shouldn't be a valid place to
start, it is, and is why I do the work to review all of the many
thousands of staging patches every release cycle.
No one is forcing you to write those patches, or read / review them,
so don't discourage others to provide them either please. I most
certainly do not.
so while i'm waxing philosophical, some other thoughts that occurred
to me that reflect on how i got started, and ways to become a more and
more useful contributor to the kernel for newbies (and i'm willing to
be corrected on any of this).
first, stop with the blind running of checkpatch unless you're
willing to take the results and examine the *context* in which they
occur. that file needs blank lines? ok, does it reside in a directory
of related files that could *also* use some blank lines? then do them
*all* -- don't waste peoples' time fixing one file at a time. if you
can do the same stylistic cleanup on an entire subsystem, go for it,
and don't clutter up the git log with numerous trivial commits.
*but* ... don't try to sneak functional changes in there at the same
time. if it's a style cleanup, then it's a *style* cleanup. one thing
at a time. but there are other places you can make a name.
first, read the Documentation/ directory -- there's lots of content
there, and quite a bit of it is out of date or just plain obsolete.
and if you want people to love you, improve the documentation. but,
see, that's going to take some work. and that's because it requires
you to read the documentation, then go off and examine the
corresponding code to see if it still matches. and why is this good?
because while updating the Documentation/ content is safe and can't
break anything, the side effect is that you *learn* about that
particular subsystem, you get some nifty patches into the kernel, and
you make lots and lots of friends.
another place to get cheap patches is to repair any kernel-doc
warnings, and there are *always* plenty of those. again, fixing
kernel-doc content shouldn't break anything, it should be easy
patching, and it normally requires you to at least examine the code to
make sure you're fixing it properly. so, you get patches into the
kernel, and you learn a bit more about some code. win-win.
last point here regarding something gregkh wrote -- yes, it's fine
to *start* with simple stylistic cleanup, especially if checkpatch
does all the work for you. but remember, that's low-hanging fruit, and
you shouldn't be greedy and try and do all of it. if stylistic cleanup
is a way for beginners to get their first patches into the kernel,
then don't be a pig and try to do it all -- leave some for others to
cut their teeth on. and what is the point of all this?
quite simply, this is also why nick krause will never be a useful
member of the kernel community. i suggested a while back that nick
could start with improving the documentation, for all the reasons i
mentioned above. his response was that he didn't know enough to do
that, which is an astonishing thing to admit. if you don't know enough
to improve the basic documentation, you have no right to be mucking
around in the code.
and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output of
checkpatch, which means he wants to grab all the trivial cleanup (the
low-hanging fruit, as it were) for himself, and not leave any for
others. rather than take the time to understand the code, nick wants
checkpatch to do all the work for him. in the end, nick doesn't want
to do any work or understand how the kernel actually works -- he just
wants patches, and he wants them as quickly and cheaply as possible.
anyway, it's time for coffee.
rday
Rday and others,
That's not what I wanted I was trying to improve my rep after getting banned from vger.org and now it seems
I can't even get a patch right. In addition I was trying to do check patch because it was easier for me
due to not understanding some parts of the code.
Nick
On Wed, Sep 17, 2014 at 5:08 PM, nick [off-list ref] wrote:
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
<snip>
quoted
anyway, it's time for coffee.
rday
Rday and others,
That's not what I wanted I was trying to improve my rep after getting banned from vger.org and now it seems
I can't even get a patch right. In addition I was trying to do check patch because it was easier for me
due to not understanding some parts of the code.
Nick
try to understand the code first. if you do not understand the code
how do you know that your patch will not break any part of the logic .
ok , by adding blank lines you will not break the logic.
but yesterday in your other patch you removed an error message . may i
ask why did you think that error message is not required ?
thanks
sudip
On Wed, Sep 17, 2014 at 5:08 PM, nick [off-list ref] wrote:
quoted
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
<snip>
quoted
quoted
anyway, it's time for coffee.
rday
Rday and others,
That's not what I wanted I was trying to improve my rep after getting banned from vger.org and now it seems
I can't even get a patch right. In addition I was trying to do check patch because it was easier for me
due to not understanding some parts of the code.
Nick
try to understand the code first. if you do not understand the code
how do you know that your patch will not break any part of the logic .
ok , by adding blank lines you will not break the logic.
but yesterday in your other patch you removed an error message . may i
ask why did you think that error message is not required ?
thanks
sudip
From: Robert P. J. Day <hidden> Date: 2014-09-17 11:53:43
what did i say? what did i just say? i wrote:
On Wed, 17 Sep 2014, nick wrote:
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
quoted
and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output
of checkpatch, which means he wants to grab all the trivial
cleanup (the low-hanging fruit, as it were) for himself, and not
leave any for others. rather than take the time to understand the
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
quoted
code, nick wants checkpatch to do all the work for him. in the
end, nick doesn't want to do any work or understand how the kernel
actually works -- he just wants patches, and he wants them as
quickly and cheaply as possible.
to which nick responds (unbelievably, and confirming what i had
just written):
That's not what I wanted I was trying to improve my rep after
getting banned from vger.org and now it seems I can't even get a
patch right. In addition I was trying to do check patch because it
was easier for me due to not understanding some parts of the code.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
i rarely have my speculation confirmed so rapidly and completely.
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
what did i say? what did i just say? i wrote:
On Wed, 17 Sep 2014, nick wrote:
quoted
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
quoted
and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output
of checkpatch, which means he wants to grab all the trivial
cleanup (the low-hanging fruit, as it were) for himself, and not
leave any for others. rather than take the time to understand the
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
quoted
quoted
code, nick wants checkpatch to do all the work for him. in the
end, nick doesn't want to do any work or understand how the kernel
actually works -- he just wants patches, and he wants them as
quickly and cheaply as possible.
to which nick responds (unbelievably, and confirming what i had
just written):
quoted
That's not what I wanted I was trying to improve my rep after
getting banned from vger.org and now it seems I can't even get a
patch right. In addition I was trying to do check patch because it
was easier for me due to not understanding some parts of the code.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
i rarely have my speculation confirmed so rapidly and completely.
rday
Rday,
I meant I didn't understand the code not the effect to write good solid patches and learn it.
Please read my messages more carefully.
Nick
On September 17, 2014 7:20:42 AM EDT, "Robert P. J. Day" [off-list ref] wrote:
<snip>
and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output of
checkpatch, which means he wants to grab all the trivial cleanup (the
low-hanging fruit, as it were) for himself, and not leave any for
others. rather than take the time to understand the code, nick wants
checkpatch to do all the work for him. in the end, nick doesn't want
to do any work or understand how the kernel actually works -- he just
wants patches, and he wants them as quickly and cheaply as possible.
Nick and his patches may have plenty of flaws, but I think it is a bit crazy to call his effort to get his first patch into the kernel greedy.
From what little I know he originally started trying to make functional code changes around various fixme's in the code. Not surprisingly, that took more overall kernel knowledge than he had. If fixme's were trivial, they would have been fixed in the first place, not a comment.
If Greg KH welcomes code style patches in the staging code as a way for newbies to learn the workflow, then that is a great thing.
Greg (not kh)
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
From: Robert P. J. Day <hidden> Date: 2014-09-17 12:00:39
On Wed, 17 Sep 2014, Greg Freemyer wrote:
On September 17, 2014 7:20:42 AM EDT, "Robert P. J. Day" [off-list ref] wrote:
<snip>
quoted
and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output of
checkpatch, which means he wants to grab all the trivial cleanup (the
low-hanging fruit, as it were) for himself, and not leave any for
others. rather than take the time to understand the code, nick wants
checkpatch to do all the work for him. in the end, nick doesn't want
to do any work or understand how the kernel actually works -- he just
wants patches, and he wants them as quickly and cheaply as possible.
Nick and his patches may have plenty of flaws, but I think it is a
bit crazy to call his effort to get his first patch into the kernel
greedy.
i was actually referring to nick's more recent posting where he
vowed to use his patch as the template to start cleaning up all of
drivers/staging/. i thought i was fairly clear that there is nothing
wrong with *starting* with stylistic cleanup, but nick made it quite
clear he planned on doing this all over drivers/staging. *that* is
what i was referring to.
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
On September 17, 2014 7:20:42 AM EDT, "Robert P. J. Day" [off-list ref] wrote:
<snip>
quoted
and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output of
checkpatch, which means he wants to grab all the trivial cleanup (the
low-hanging fruit, as it were) for himself, and not leave any for
others. rather than take the time to understand the code, nick wants
checkpatch to do all the work for him. in the end, nick doesn't want
to do any work or understand how the kernel actually works -- he just
wants patches, and he wants them as quickly and cheaply as possible.
Nick and his patches may have plenty of flaws, but I think it is a bit crazy to call his effort to get his first patch into the kernel greedy.
quoted
From what little I know he originally started trying to make functional code changes around various fixme's in the code. Not surprisingly, that took more overall kernel knowledge than he had. If fixme's were trivial, they would have been fixed in the first place, not a comment.
If Greg KH welcomes code style patches in the staging code as a way for newbies to learn the workflow, then that is a great thing.
Greg (not kh)
Thanks Greg,
That is exactly what happened with my issues, I started on things with more then I could handle and due to that and not listening was banned. If someone with re look at my patch and tell if it's OK or not, if it's good please sent
it off , if not I would like to known exactly where I am wrong so I can learn.
Nick
On September 17, 2014 7:20:42 AM EDT, "Robert P. J. Day" [off-list ref] wrote:
<snip>
quoted
and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output of
checkpatch, which means he wants to grab all the trivial cleanup (the
low-hanging fruit, as it were) for himself, and not leave any for
others. rather than take the time to understand the code, nick wants
checkpatch to do all the work for him. in the end, nick doesn't want
to do any work or understand how the kernel actually works -- he just
wants patches, and he wants them as quickly and cheaply as possible.
Nick and his patches may have plenty of flaws, but I think it is a
bit crazy to call his effort to get his first patch into the kernel
greedy.
i was actually referring to nick's more recent posting where he
vowed to use his patch as the template to start cleaning up all of
drivers/staging/. i thought i was fairly clear that there is nothing
wrong with *starting* with stylistic cleanup, but nick made it quite
clear he planned on doing this all over drivers/staging. *that* is
what i was referring to.
rday
Rday,
Your reading that wrong what I mean is to use the format as a template for patches I am going to send out,
not clean up drivers/staging all of it a least. I was stating I wanted to only clean up a bit there in order
to get comfortable with sending out patches.
Sorry about the miswritten message,
Nick
I thought that the return statement of NULL to a caller was enough.
Nick
Uh...
I don't know that chunk of code, but error messages that go to the kernel log exist for a specific reason. Taking them out requires a specific reason.
Ie. This would make a good commit message "At this point the condition is well understood and the code that handles it is well tested and has been stable for 3 years, thus removing the error message."
Greg
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
I thought that the return statement of NULL to a caller was enough.
Nick
Uh...
I don't know that chunk of code, but error messages that go to the kernel log exist for a specific reason. Taking them out requires a specific reason.
Ie. This would make a good commit message "At this point the condition is well understood and the code that handles it is well tested and has been stable for 3 years, thus removing the error message."
Greg
Thanks Greg Again,
This is what I meant with my patch, why have a unneeded error message if the code is already tested and only uses
the return value in that function.
Cheers Nick
From: Chris Lee <hidden> Date: 2014-09-17 12:17:09
Rday,
I meant I didn't understand the code not the effect to write good solid
patches and learn it.
Please read my messages more carefully.
Nick
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
From: Kai Bojens <hidden> Date: 2014-09-17 12:17:37
On 17-09-14 08:09:36, nick wrote:
[Again quoting everything]
Please read and understand this:
-> http://en.wikipedia.org/wiki/Posting_style#How_much_to_trim
Your replies are unreadable to me as I don't intend to scroll down
several pages just to read the one line you added as an answer. If
you are seriously interested in any help (I for one doubt this?) you
should start writing in a more readable way.
Rday,
I meant I didn't understand the code not the effect to write good solid
patches and learn it.
Please read my messages more carefully.
Nick
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Terrible reason to submit a patch. If you dont understand the code snippet,
you should not be submitting patch's against them. You need to fully
understand the code inside and out before you even consider fixing it.
Chris
Thanks Chris,
I agree with you now. I didn't mean about check patch for your information but on other parts.
Thanks Nick
On 17-09-14 08:09:36, nick wrote:
[Again quoting everything]
Please read and understand this:
-> http://en.wikipedia.org/wiki/Posting_style#How_much_to_trim
Your replies are unreadable to me as I don't intend to scroll down
several pages just to read the one line you added as an answer. If
you are seriously interested in any help (I for one doubt this?) you
should start writing in a more readable way.
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
I sent out my last email fine. Your thinking of another email and yes I really would like some help as if I learn more I will be able to a good kernel member with a tutor or some advice.
Nick
I thought that the return statement of NULL to a caller was enough.
Nick
Uh...
I don't know that chunk of code, but error messages that go to the
kernel log exist for a specific reason. Taking them out requires a
specific reason.
quoted
Ie. This would make a good commit message "At this point the
condition is well understood and the code that handles it is well
tested and has been stable for 3 years, thus removing the error
message."
quoted
Greg
Thanks Greg Again,
This is what I meant with my patch, why have a unneeded error message
if the code is already tested and only uses
the return value in that function.
Cheers Nick
Because in general we don't use asserts in the kernel. I'm sure I've used 10,000s of asserts in user space over the decades. Zero in the kernel.
Specifically, in user space when writing code we can put asserts throughout the code that will cause an immediate code explosion if unexpected things happen. In the kernel, the better choice is printing an error message then have the code do it's best to handle it.
That still begs the question of why it happened in the first place. As long as the event itself us unexpected (ie. not routine) then the error message should remain. Re-read the sample commit message I wrote. The first thing I said is the "condition is well understood". Never remove an error message unless you can explain with clarity why the "error" is happening. Obviously in that case you should be replacing the error message with a comment that explains the condition.
Greg
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
From: Nick Krause <hidden> Date: 2014-09-17 12:29:20
Because in general we don't use asserts in the kernel. I'm sure I've used 10,000s of asserts in user space over the decades. Zero in the kernel.
Specifically, in user space when writing code we can put asserts throughout the code that will cause an immediate code explosion if unexpected things happen. In the kernel, the better choice is printing an error message then have the code do it's best to handle it.
That still begs the question of why it happened in the first place. As long as the event itself us unexpected (ie. not routine) then the error message should remain. Re-read the sample commit message I wrote. The first thing I said is the "condition is well understood". Never remove an error message unless you can explain with clarity why the "error" is happening. Obviously in that case you should be replacing the error message with a comment that explains the condition.
Greg
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
Thanks Greg,
I will look into in more carefully later. In addition thanks to all
the others for the patience and help. I understand that
this is not normal in the kernel community and would like to really
thank everyone for the patience and support. I
want to help out and as I am finding out the coding is not the issue
it's my issues with the community which I hope
we can fix in order for me to help the kernel community. In addition I
do find the kernel interesting and really like
working with it, just having issues with understanding how to write patches.
Nick