A quick guide to why stand-alone checkpatch patches suck...
In general, stand-alone patches to "fix" checkpatch whining are a Bad Idea(TM). Here's why... First off, the type of programmer who is tempted to do checkpatch cleanup as "My First Kernel Patch" are, by and large, novices. The code in the kernel falls into one of several states of use and stability. Some code is old and heavily used, some is old and not used, and some of it is under active development. Let's look at each case. There's parts of the kernel that have been around for *years* and are beat on constantly - the VFS, the driver core code, large parts of the network stack for example. Stand-alone checkpatch fixes here aren't a good idea, because they're potentially destabilizing if somebody gets the fix wrong. And yes, this has happened, and checkpatch "fixes" have actually introduced bugs - it's part of why there's a "one thing, one patch" rule, to make it easier to audit a patch and ensure it doesn't introduce regressions. Oh, and most of this sort of code is already *really* clean, because professionals have cleaned it up. There's parts of the kernel that are digital archaeology waiting to happen (I'm looking at you, floppy.c :). Again, fixing style is probably a Bad Idea, because (a) you might introduce a bug and (b) nobody is even looking at this code anymore, so nobody *cares* about the style. And if it's an abandoned part of the code, there may be nobody who cares/understands it well enough to notice if a subtle bug is introduced. So that leaves us code that's under active development. And here, checkpatch fixes are actually a *detriment*, and they tick the subsystem maintainer off. That's because they have a high probability of causing merge conflicts with somebody else's patches that are doing *actual code improvement*. The end result? Unless you *are* that "somebody else" who's doing other work on the code, you shouldn't submit checkpatch cleanups. So we shouldn't see these from anybody except as prep work for actual coding. (But by all means, if you're about to apply a can-opener to a .c file to do some Real Work, feel free to spin a preparatory set of cleanup patches so you're performing code surgery on a properly scrubbed, prepped, and anesthetized patient :)
On Tue, Sep 16, 2014 at 08:25:51PM -0400, Valdis Kletnieks wrote:
In general, stand-alone patches to "fix" checkpatch whining are a Bad Idea(TM).
<snip> And here's why checkpatch patches are a "Good Idea(TM)": - it teaches you how to set up your email client properly - it teaches you how to describe a patch properly - it teaches you how to submit a patch properly - it gives you a good feedback loop - it is an "easy" place to start. But, and this is a huge BUT, which you ignored, you should ONLY submit a checkpatch cleanup for a subsystem that has a maintainer that welcomes them. Like drivers/staging/*. Checkpatch cleanups are welcome, and encouraged there. If you want to do a checkpatch cleanup, do it there, you will not be yelled at. thanks, greg k-h
On Tue, Sep 16, 2014 at 7:25 PM, Valdis Kletnieks <Valdis.Kletnieks@vt.edu> wrote:
In general, stand-alone patches to "fix" checkpatch whining are a Bad Idea(TM).
That's just YOUR opinion. GregKH actually made a presentation to help us n00bs do exactly that: https://www.youtube.com/watch?v=LLBrBBImJt4 And he has been applying my cleanup patches for nearly a month now: http://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/log/?h=stagin...
First off, the type of programmer who is tempted to do checkpatch cleanup as "My First Kernel Patch" are, by and large, novices.
OMG, I'm a novice.. cat's out of the bag! Your point?
There's parts of the kernel that have been around for *years* and are beat on constantly - the VFS, the driver core code, large parts of the network stack for example. Stand-alone checkpatch fixes here aren't a good idea, because they're potentially destabilizing if somebody gets the fix wrong. And yes, this has happened, and checkpatch "fixes" have actually introduced bugs - it's part of why there's a "one thing, one patch" rule, to make it easier to audit a patch and ensure it doesn't introduce regressions. Oh, and most of this sort of code is already *really* clean, because professionals have cleaned it up.
fs/* currently contains 96,375 errors and 22,555 warnings. net/* currently contains 3,366 errors and 19,536 warnings. *really* clean? LOL.
So that leaves us code that's under active development. And here, checkpatch fixes are actually a *detriment*, and they tick the subsystem maintainer off. That's because they have a high probability of causing merge conflicts with somebody else's patches that are doing *actual code improvement*.
So? Either you suck it up and re-spin your patch or you go home and cry.
The end result? Unless you *are* that "somebody else" who's doing other work on the code, you shouldn't submit checkpatch cleanups.
I do Kernel janitor work for the *fun* of it. I program in Python, Java, and ObjC and manage a bunch of Linux servers at my day job.. so changing up and doing Kernel cleanups in my spare time is *fun*. I may or may not ever actually become a full-fledged Kernel hacker and I don't really care either way. Meanwhile drivers/staging/* contains 19,004 errors and 35,292 warnings and until the man stops taking my patches I'm going to continue sending them. As for you I'm just gonna ignore you and your discouraging posts from here on. It's not like you're in MAINTAINERS or anything. -- Greg Donald
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....)
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks@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 <greg@kroah.com> wrote:
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks@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.)
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@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
On Tue, 16 Sep 2014, Greg KH wrote:
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks@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.
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 ========================================================================
On Tue, 16 Sep 2014, Greg KH wrote:
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks@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.
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 14-09-17 07:20 AM, Robert P. J. Day wrote:
On Tue, 16 Sep 2014, Greg KH wrote:
On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks@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.
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 <xerofoify@gmail.com> wrote:
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
<snip>
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
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
On 14-09-17 07:51 AM, Sudip Mukherjee wrote:
On Wed, Sep 17, 2014 at 5:08 PM, nick <xerofoify@gmail.com> wrote:
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
<snip>
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
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies I thought that the return statement of NULL to a caller was enough. Nick
On September 17, 2014 7:53:24 AM EDT, nick <xerofoify@gmail.com> wrote:
On 14-09-17 07:51 AM, Sudip Mukherjee wrote:
On Wed, Sep 17, 2014 at 5:08 PM, nick <xerofoify@gmail.com> wrote:
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
<snip>
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
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies 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.
On 14-09-17 08:05 AM, Greg Freemyer wrote:
On September 17, 2014 7:53:24 AM EDT, nick <xerofoify@gmail.com> wrote:
On 14-09-17 07:51 AM, Sudip Mukherjee wrote:
On Wed, Sep 17, 2014 at 5:08 PM, nick <xerofoify@gmail.com> wrote:
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
<snip>
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
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies 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
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.
On 14-09-17 08:17 AM, Kai Bojens wrote:
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@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
On September 17, 2014 8:09:36 AM EDT, nick <xerofoify@gmail.com> wrote:
On 14-09-17 08:05 AM, Greg Freemyer wrote:
On September 17, 2014 7:53:24 AM EDT, nick <xerofoify@gmail.com>
wrote:
On 14-09-17 07:51 AM, Sudip Mukherjee wrote:
On Wed, Sep 17, 2014 at 5:08 PM, nick <xerofoify@gmail.com> wrote:
On 14-09-17 07:20 AM, Robert P. J. Day wrote:
<snip>
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
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies 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
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.
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
On Wed, 17 Sep 2014 08:09:36 -0400, nick said:
On 14-09-17 08:05 AM, Greg Freemyer wrote:
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."
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.
Sorry Nick, but that's *not* what Greg meant. What he *meant* was that removal of an error message should be its *own* commit, explaining *why* it was being removed and why it was OK to do so. He did *not* say that this particular removal was in fact correct. He *did* say that such a hypothetical removal *should not* be in a patch calling itself a checkpatch cleanup. He *did* say that the patch will require proof that you've examined the code, understood it, and can explain *why* the patch is OK.
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:
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.
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 ========================================================================
On 14-09-17 07:53 AM, Robert P. J. Day wrote:
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:
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.
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
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
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@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
On 14-09-17 08:17 AM, Chris Lee wrote:
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@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 September 17, 2014 7:20:42 AM EDT, "Robert P. J. Day" <rpjday@crashcourse.ca> 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.
On Wed, 17 Sep 2014, Greg Freemyer wrote:
On September 17, 2014 7:20:42 AM EDT, "Robert P. J. Day" <rpjday@crashcourse.ca> 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.
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 14-09-17 08:00 AM, Robert P. J. Day wrote:
On Wed, 17 Sep 2014, Greg Freemyer wrote:
On September 17, 2014 7:20:42 AM EDT, "Robert P. J. Day" <rpjday@crashcourse.ca> 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.
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
On 14-09-17 07:56 AM, Greg Freemyer wrote:
On September 17, 2014 7:20:42 AM EDT, "Robert P. J. Day" <rpjday@crashcourse.ca> 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)
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 Wed, Sep 17, 2014 at 10:39 AM, <Valdis.Kletnieks@vt.edu> wrote:
On Wed, 17 Sep 2014 08:02:01 -0400, nick said:
it off , if not I would like to known exactly where I am wrong so I can learn.
Somebody wake me up when he actually *means* that.
Valdis, I understand that was what he stated I was looking into how to write a correct patch not remove needed error messages. Nick
This was a great discussion, until you guys started feeding the troll again. Can we just stop feeding the troll? He will prob go way... If every *single* time that Nick posts something you reply to him it will only lead to more replies from him saying he will get better, understand what he is doing and that he wants to improve his rep with the community. That's his standard speech. BGS
On Wed, Sep 17, 2014 at 1:13 PM, Bruno Guedes Souto <brunoguedes@gmail.com> wrote:
This was a great discussion, until you guys started feeding the troll again.
Can we just stop feeding the troll? He will prob go way...
If every *single* time that Nick posts something you reply to him it will only lead to more replies from him saying he will get better, understand what he is doing and that he wants to improve his rep with the community. That's his standard speech.
BGS
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies Can I stop being called a troll. I am trying to learn here and improve my rep and understanding of the kernel. I would like to help but with this negative light around me it is much more difficult and hard to do. I understand the problems I have caused now and would like to improve my rep with a tutor or someone who is willing to be my router to the community until I improve enough to a member of the community on my own. Thanks Nick
Am 2014-09-17 19:47, schrieb Nick Krause:
On Wed, Sep 17, 2014 at 1:13 PM, Bruno Guedes Souto <brunoguedes@gmail.com> wrote:
This was a great discussion, until you guys started feeding the troll again.
Can we just stop feeding the troll? He will prob go way...
If every *single* time that Nick posts something you reply to him it will only lead to more replies from him saying he will get better, understand what he is doing and that he wants to improve his rep with the community. That's his standard speech.
BGS
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies Can I stop being called a troll. I am trying to learn here and improve my rep and understanding of the kernel. I would like to help but with this negative light around me it is much more difficult and hard to do. I understand the problems I have caused now and would like to improve my rep with a tutor or someone who is willing to be my router to the community until I improve enough to a member of the community on my own. Thanks Nick
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies Why do you think that some stranger on the internet is going to be your tutor? Unless you belong to some minority, you shouldn't be waiting for a tutor to take your hand and guide you... that's probably not going to happen. Apply the advice that's been already given to you, e.g. read the old mails and write each advice down. Until that, *please* lurk moar.
e stranger on the internet is going to be your tutor? Unless you belong to some minority, you shouldn't be waiting for a tutor to take your hand and guide you... that's probably not going to happen. Apply the advice that's been already given to you, e.g. read the old mails and write each advice down. Until that, *please* lurk moar. On Wed, Sep 17, 2014 at 2:01 PM, Philipp Muhoray <philipp.muhoray@gmail.com> wrote:
Am 2014-09-17 19:47, schrieb Nick Krause:
On Wed, Sep 17, 2014 at 1:13 PM, Bruno Guedes Souto <brunoguedes@gmail.com> wrote:
This was a great discussion, until you guys started feeding the troll again.
Can we just stop feeding the troll? He will prob go way...
If every *single* time that Nick posts something you reply to him it will only lead to more replies from him saying he will get better, understand what he is doing and that he wants to improve his rep with the community. That's his standard speech.
BGS
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbe stranger on the internet is going to be your
tutor? Unless you belong to some minority, you shouldn't be waiting for a tutor to take your hand and guide you... that's probably not going to happen. Apply the advice that's been already given to you, e.g. read the old mails and write each advice down. Until that, *please* lurk moar. ies
Can I stop being called a troll. I am trying to learn here and improve my rep and understanding of the kernel. I would like to help but with this negative light around me it is much more difficult and hard to do. I understand the problems I have caused now and would like to improve my rep with a tutor or someone who is willing to be my router to the community until I improve enough to a member of the community on my own. Thanks Nick
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies Why do you think that some stranger on the internet is going to be your tutor? Unless you belong to some minority, you shouldn't be waiting for a tutor to take your hand and guide you... that's probably not going to happen. Apply the advice that's been already given to you, e.g. read the old mails and write each advice down. Until that, *please* lurk moar.
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies That's what I plan to do now and ask many questions. Nick
On Wed, 17 Sep 2014, Nick Krause wrote:
... like to improve my rep with a tutor or someone who is willing to be my router to the community ...
there is no sane human being that would offer to be a tutor or mentor (a request you've made before) or "router" to someone who absolutely refuses to listen to the advice people give him. you have absolutely no idea what mentoring involves, do you, nick? people offer to become mentors because they find up and coming folks who appear to be bright, ambitious and teachable, not irredeemably thick. this *entire* *mailing* *list* has been trying to tutor or mentor you for the last two months, and it has been a colossal waste of time. where do you get the nerve to now ask for *personal* assistance, given that everyone knows you simply ignore whatever anyone tells you? can we please, fer chrissake, just vote on a ban to get rid of nick and cut the nonsense on this mailing list by 90%? please? rday -- ======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
On Wed, Sep 17, 2014 at 2:15 PM, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
On Wed, 17 Sep 2014, Nick Krause wrote:
... like to improve my rep with a tutor or someone who is willing to be my router to the community ...
there is no sane human being that would offer to be a tutor or mentor (a request you've made before) or "router" to someone who absolutely refuses to listen to the advice people give him.
you have absolutely no idea what mentoring involves, do you, nick? people offer to become mentors because they find up and coming folks who appear to be bright, ambitious and teachable, not irredeemably thick.
this *entire* *mailing* *list* has been trying to tutor or mentor you for the last two months, and it has been a colossal waste of time. where do you get the nerve to now ask for *personal* assistance, given that everyone knows you simply ignore whatever anyone tells you?
can we please, fer chrissake, just vote on a ban to get rid of nick and cut the nonsense on this mailing list by 90%? please?
rday
--
======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca
Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
I am leaving for now and going to learn more about the kernel and come back with some questions later. Nick
participants (14)
-
Bruno Guedes Souto -
Chris Lee -
Greg Donald -
Greg Freemyer -
Greg KH -
John de la Garza -
Kai Bojens -
nick -
Nick Krause
-
Philipp Muhoray -
Robert P. J. Day -
Sudip Mukherjee -
Valdis Kletnieks -
Valdis.Kletnieks＠vt.edu