Re: A quick guide to why stand-alone checkpatch patches suck...
On Tue, 16 Sep 2014, Greg KH wrote: On Tue, Sep 16, 2014 at 11:42:18PM -0400, valdis.kletni...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Tue, 16 Sep 2014, Greg KH wrote: On Tue, Sep 16, 2014 at 11:42:18PM -0400, valdis.kletni...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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.kletni...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Wed, Sep 17, 2014 at 5:08 PM, nick xerofo...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On 14-09-17 07:51 AM, Sudip Mukherjee wrote: On Wed, Sep 17, 2014 at 5:08 PM, nick xerofo...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On September 17, 2014 7:20:42 AM EDT, Robert P. J. Day rpj...@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. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Wed, 17 Sep 2014, Greg Freemyer wrote: On September 17, 2014 7:20:42 AM EDT, Robert P. J. Day rpj...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On 14-09-17 07:56 AM, Greg Freemyer wrote: On September 17, 2014 7:20:42 AM EDT, Robert P. J. Day rpj...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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 rpj...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On September 17, 2014 7:53:24 AM EDT, nick xerofo...@gmail.com wrote: On 14-09-17 07:51 AM, Sudip Mukherjee wrote: On Wed, Sep 17, 2014 at 5:08 PM, nick xerofo...@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. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On 14-09-17 08:05 AM, Greg Freemyer wrote: On September 17, 2014 7:53:24 AM EDT, nick xerofo...@gmail.com wrote: On 14-09-17 07:51 AM, Sudip Mukherjee wrote: On Wed, Sep 17, 2014 at 5:08 PM, nick xerofo...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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
Re: A quick guide to why stand-alone checkpatch patches suck...
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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On September 17, 2014 8:09:36 AM EDT, nick xerofo...@gmail.com wrote: On 14-09-17 08:05 AM, Greg Freemyer wrote: On September 17, 2014 7:53:24 AM EDT, nick xerofo...@gmail.com wrote: On 14-09-17 07:51 AM, Sudip Mukherjee wrote: On Wed, Sep 17, 2014 at 5:08 PM, nick xerofo...@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. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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. pgpLUVHigG6xc.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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. pgpep_zcvBkMR.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Wed, Sep 17, 2014 at 10:39 AM, valdis.kletni...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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
Re: A quick guide to why stand-alone checkpatch patches suck...
On Wed, Sep 17, 2014 at 1:13 PM, Bruno Guedes Souto brunogue...@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
Re: A quick guide to why stand-alone checkpatch patches suck...
Am 2014-09-17 19:47, schrieb Nick Krause: On Wed, Sep 17, 2014 at 1:13 PM, Bruno Guedes Souto brunogue...@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. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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.muho...@gmail.com wrote: Am 2014-09-17 19:47, schrieb Nick Krause: On Wed, Sep 17, 2014 at 1:13 PM, Bruno Guedes Souto brunogue...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Wed, Sep 17, 2014 at 2:15 PM, Robert P. J. Day rpj...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Wed, Sep 17, 2014 at 05:13:58PM +, Bruno Guedes Souto wrote: This was a great discussion, until you guys started feeding the troll again. It seems like a lot of people here don't get it and continue to keep basically repeating themselves. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
I wouldn't touch those with a ten foot pole. On 16 Sep 2014 14:46, nick xerofo...@gmail.com wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. Thanks, Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On 14-09-16 08:47 AM, Kristofer Hallin wrote: I wouldn't touch those with a ten foot pole. On 16 Sep 2014 14:46, nick xerofo...@gmail.com wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. Thanks, Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies Kristofer, I would appreciate if you look at them and see if there any good. I understand based on my other patches so assumptions but it would be nice if you only too a look for me. Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Checkpatch Patches
I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. Thanks, Nick From 7bf4229fa2f9c4fcf3243bc738c74bfdc58a6594 Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Sat, 13 Sep 2014 11:53:24 -0400 Subject: [PATCH] staging wlan-ng: Add missing a blank line after declarations Fixing trivial checkpatch warnings about missing line after declarations. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- Tested by compilation only. drivers/staging/wlan-ng/hfa384x.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 1f2c78c..20d146b 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -1376,6 +1376,7 @@ int hfa384x_drvr_setconfig(hfa384x_t *hw, u16 rid, void *buf, u16 len); static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) { int result = 0; + result = hfa384x_drvr_getconfig(hw, rid, val, sizeof(u16)); if (result == 0) *((u16 *) val) = le16_to_cpu(*((u16 *) val)); @@ -1385,6 +1386,7 @@ static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) static inline int hfa384x_drvr_setconfig16(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig(hw, rid, value, sizeof(value)); } @@ -1402,6 +1404,7 @@ static inline int hfa384x_drvr_setconfig16_async(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig_async(hw, rid, value, sizeof(value), NULL, NULL); } 1.9.1 From 5eb3de22f0760ece1e838d48c8dd9148b4331cdc Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Mon, 15 Sep 2014 17:07:24 -0400 Subject: [PATCH] staging netlogic: Fix checkpatch errors in xlr_net.c This removes the checkpatch errors related to a needed line below declaration of a struct and another about a non nessary printk message about a NULL allocated skb due to the function returning NULL to the caller of the function and the printk no longer being' used or needed by any callers. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- drivers/staging/netlogic/xlr_net.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c index 9bf407d..28a42831 100644 --- a/drivers/staging/netlogic/xlr_net.c +++ b/drivers/staging/netlogic/xlr_net.c @@ -142,10 +142,8 @@ static inline struct sk_buff *xlr_alloc_skb(void) /* skb-data is cache aligned */ skb = alloc_skb(XLR_RX_BUF_SIZE, GFP_ATOMIC); - if (!skb) { - pr_err(SKB allocation failed\n); + if (!skb) return NULL; - } mac_put_skb_back_ptr(skb); return skb; } @@ -1104,6 +1102,7 @@ err_gmac: static int xlr_net_remove(struct platform_device *pdev) { struct xlr_net_priv *priv = platform_get_drvdata(pdev); + unregister_netdev(priv-ndev); mdiobus_unregister(priv-mii_bus); mdiobus_free(priv-mii_bus); -- 1.9.1 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014, nick wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. hey, kids ... remember what i wrote recently about, at the very least, having a grammatically correct subject line? well, there you go. why nick has not been banned from this list is purely a mystery at this point. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014, nick wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. oh, and remember how i wrote *explicitly* that the patch message should match the properties of the patch, and that one should not claim to be fixing an error if what is being rectified is merely a *warning*? remember i wrote that? take a wild guess as to what nick did in his second patch. go ahead, guess. you'll never guess. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, Sep 16, 2014 at 08:44:27AM -0400, nick wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. Thanks, Nick From 7bf4229fa2f9c4fcf3243bc738c74bfdc58a6594 Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Sat, 13 Sep 2014 11:53:24 -0400 Subject: [PATCH] staging wlan-ng: Add missing a blank line after declarations Fixing trivial checkpatch warnings about missing line after declarations. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- Tested by compilation only. drivers/staging/wlan-ng/hfa384x.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 1f2c78c..20d146b 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -1376,6 +1376,7 @@ int hfa384x_drvr_setconfig(hfa384x_t *hw, u16 rid, void *buf, u16 len); static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) { int result = 0; + result = hfa384x_drvr_getconfig(hw, rid, val, sizeof(u16)); if (result == 0) *((u16 *) val) = le16_to_cpu(*((u16 *) val)); @@ -1385,6 +1386,7 @@ static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) static inline int hfa384x_drvr_setconfig16(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig(hw, rid, value, sizeof(value)); } @@ -1402,6 +1404,7 @@ static inline int hfa384x_drvr_setconfig16_async(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig_async(hw, rid, value, sizeof(value), NULL, NULL); } 1.9.1 From 5eb3de22f0760ece1e838d48c8dd9148b4331cdc Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Mon, 15 Sep 2014 17:07:24 -0400 Subject: [PATCH] staging netlogic: Fix checkpatch errors in xlr_net.c This removes the checkpatch errors related to a needed line below declaration of a struct and another about a non nessary printk message about a NULL allocated skb due to the function returning NULL to the caller of the function and the printk no longer being' used or needed by any callers. spelling mistakes Signed-off-by: Nicholas Krause xerofo...@gmail.com --- drivers/staging/netlogic/xlr_net.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c index 9bf407d..28a42831 100644 --- a/drivers/staging/netlogic/xlr_net.c +++ b/drivers/staging/netlogic/xlr_net.c @@ -142,10 +142,8 @@ static inline struct sk_buff *xlr_alloc_skb(void) /* skb-data is cache aligned */ skb = alloc_skb(XLR_RX_BUF_SIZE, GFP_ATOMIC); - if (!skb) { - pr_err(SKB allocation failed\n); why the error message was removed ? + if (!skb) return NULL; - } mac_put_skb_back_ptr(skb); return skb; } @@ -1104,6 +1102,7 @@ err_gmac: static int xlr_net_remove(struct platform_device *pdev) { struct xlr_net_priv *priv = platform_get_drvdata(pdev); + unregister_netdev(priv-ndev); mdiobus_unregister(priv-mii_bus); mdiobus_free(priv-mii_bus); -- 1.9.1 ohhh .. yeah .. and both the patch failed when i tried to apply them to next-20140916 why are we wasting our time for your patches , which are bound to have some problem thanks sudip ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, Sep 16, 2014 at 7:16 PM, Sudip Mukherjee sudipm.mukher...@gmail.com wrote: On Tue, Sep 16, 2014 at 7:02 PM, nick xerofo...@gmail.com wrote: On 14-09-16 09:28 AM, Sudip Mukherjee wrote: On Tue, Sep 16, 2014 at 6:56 PM, nick xerofo...@gmail.com wrote: On 14-09-16 09:21 AM, Sudip Mukherjee wrote: On Tue, Sep 16, 2014 at 6:41 PM, nick xerofo...@gmail.com wrote: On 14-09-16 09:06 AM, Sudip Mukherjee wrote: On Tue, Sep 16, 2014 at 08:44:27AM -0400, nick wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. Thanks, Nick From 7bf4229fa2f9c4fcf3243bc738c74bfdc58a6594 Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Sat, 13 Sep 2014 11:53:24 -0400 Subject: [PATCH] staging wlan-ng: Add missing a blank line after declarations Fixing trivial checkpatch warnings about missing line after declarations. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- Tested by compilation only. drivers/staging/wlan-ng/hfa384x.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 1f2c78c..20d146b 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -1376,6 +1376,7 @@ int hfa384x_drvr_setconfig(hfa384x_t *hw, u16 rid, void *buf, u16 len); static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) { int result = 0; + result = hfa384x_drvr_getconfig(hw, rid, val, sizeof(u16)); if (result == 0) *((u16 *) val) = le16_to_cpu(*((u16 *) val)); @@ -1385,6 +1386,7 @@ static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) static inline int hfa384x_drvr_setconfig16(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig(hw, rid, value, sizeof(value)); } @@ -1402,6 +1404,7 @@ static inline int hfa384x_drvr_setconfig16_async(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig_async(hw, rid, value, sizeof(value), NULL, NULL); } 1.9.1 From 5eb3de22f0760ece1e838d48c8dd9148b4331cdc Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Mon, 15 Sep 2014 17:07:24 -0400 Subject: [PATCH] staging netlogic: Fix checkpatch errors in xlr_net.c This removes the checkpatch errors related to a needed line below declaration of a struct and another about a non nessary printk message about a NULL allocated skb due to the function returning NULL to the caller of the function and the printk no longer being' used or needed by any callers. spelling mistakes Signed-off-by: Nicholas Krause xerofo...@gmail.com --- drivers/staging/netlogic/xlr_net.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c index 9bf407d..28a42831 100644 --- a/drivers/staging/netlogic/xlr_net.c +++ b/drivers/staging/netlogic/xlr_net.c @@ -142,10 +142,8 @@ static inline struct sk_buff *xlr_alloc_skb(void) /* skb-data is cache aligned */ skb = alloc_skb(XLR_RX_BUF_SIZE, GFP_ATOMIC); -if (!skb) { -pr_err(SKB allocation failed\n); why the error message was removed ? +if (!skb) return NULL; -} mac_put_skb_back_ptr(skb); return skb; } @@ -1104,6 +1102,7 @@ err_gmac: static int xlr_net_remove(struct platform_device *pdev) { struct xlr_net_priv *priv = platform_get_drvdata(pdev); + unregister_netdev(priv-ndev); mdiobus_unregister(priv-mii_bus); mdiobus_free(priv-mii_bus); -- 1.9.1 ohhh .. yeah .. and both the patch failed when i tried to apply them to next-20140916 why are we wasting our time for your patches , which are bound to have some problem thanks sudip ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies I tried these on Greg's tree of staging-next and they worked for me. Nick in drivers/staging/netlogic/xlr_net.c file of next-20140916 1) in line 142 we just have an } else { (doesnot match your patch) 2) in the same file , there is only one instance of pr_err(SKB allocation failed\n); , and that is at line 208 , and that is followed by a return -ENOMEM; , but your patch is showing that there is a return NULL thanks sudip Thanks Sudip, I will fix the patch later if you want. In addition is my other patch OK or do I need to fix it still? Nick allow me to quote from my mail : ohhh .. yeah .. and both the patch failed when i tried to apply them to next-20140916 i mentioned both ... Thanks for the Help, I really do appreciate it and I
Re: Checkpatch Patches
On 14-09-16 09:21 AM, Sudip Mukherjee wrote: On Tue, Sep 16, 2014 at 6:41 PM, nick xerofo...@gmail.com wrote: On 14-09-16 09:06 AM, Sudip Mukherjee wrote: On Tue, Sep 16, 2014 at 08:44:27AM -0400, nick wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. Thanks, Nick From 7bf4229fa2f9c4fcf3243bc738c74bfdc58a6594 Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Sat, 13 Sep 2014 11:53:24 -0400 Subject: [PATCH] staging wlan-ng: Add missing a blank line after declarations Fixing trivial checkpatch warnings about missing line after declarations. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- Tested by compilation only. drivers/staging/wlan-ng/hfa384x.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 1f2c78c..20d146b 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -1376,6 +1376,7 @@ int hfa384x_drvr_setconfig(hfa384x_t *hw, u16 rid, void *buf, u16 len); static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) { int result = 0; + result = hfa384x_drvr_getconfig(hw, rid, val, sizeof(u16)); if (result == 0) *((u16 *) val) = le16_to_cpu(*((u16 *) val)); @@ -1385,6 +1386,7 @@ static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) static inline int hfa384x_drvr_setconfig16(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig(hw, rid, value, sizeof(value)); } @@ -1402,6 +1404,7 @@ static inline int hfa384x_drvr_setconfig16_async(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig_async(hw, rid, value, sizeof(value), NULL, NULL); } 1.9.1 From 5eb3de22f0760ece1e838d48c8dd9148b4331cdc Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Mon, 15 Sep 2014 17:07:24 -0400 Subject: [PATCH] staging netlogic: Fix checkpatch errors in xlr_net.c This removes the checkpatch errors related to a needed line below declaration of a struct and another about a non nessary printk message about a NULL allocated skb due to the function returning NULL to the caller of the function and the printk no longer being' used or needed by any callers. spelling mistakes Signed-off-by: Nicholas Krause xerofo...@gmail.com --- drivers/staging/netlogic/xlr_net.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c index 9bf407d..28a42831 100644 --- a/drivers/staging/netlogic/xlr_net.c +++ b/drivers/staging/netlogic/xlr_net.c @@ -142,10 +142,8 @@ static inline struct sk_buff *xlr_alloc_skb(void) /* skb-data is cache aligned */ skb = alloc_skb(XLR_RX_BUF_SIZE, GFP_ATOMIC); -if (!skb) { -pr_err(SKB allocation failed\n); why the error message was removed ? +if (!skb) return NULL; -} mac_put_skb_back_ptr(skb); return skb; } @@ -1104,6 +1102,7 @@ err_gmac: static int xlr_net_remove(struct platform_device *pdev) { struct xlr_net_priv *priv = platform_get_drvdata(pdev); + unregister_netdev(priv-ndev); mdiobus_unregister(priv-mii_bus); mdiobus_free(priv-mii_bus); -- 1.9.1 ohhh .. yeah .. and both the patch failed when i tried to apply them to next-20140916 why are we wasting our time for your patches , which are bound to have some problem thanks sudip ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies I tried these on Greg's tree of staging-next and they worked for me. Nick in drivers/staging/netlogic/xlr_net.c file of next-20140916 1) in line 142 we just have an } else { (doesnot match your patch) 2) in the same file , there is only one instance of pr_err(SKB allocation failed\n); , and that is at line 208 , and that is followed by a return -ENOMEM; , but your patch is showing that there is a return NULL thanks sudip Thanks Sudip, I will fix the patch later if you want. In addition is my other patch OK or do I need to fix it still? Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On 14-09-16 09:06 AM, Sudip Mukherjee wrote: On Tue, Sep 16, 2014 at 08:44:27AM -0400, nick wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. Thanks, Nick From 7bf4229fa2f9c4fcf3243bc738c74bfdc58a6594 Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Sat, 13 Sep 2014 11:53:24 -0400 Subject: [PATCH] staging wlan-ng: Add missing a blank line after declarations Fixing trivial checkpatch warnings about missing line after declarations. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- Tested by compilation only. drivers/staging/wlan-ng/hfa384x.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 1f2c78c..20d146b 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -1376,6 +1376,7 @@ int hfa384x_drvr_setconfig(hfa384x_t *hw, u16 rid, void *buf, u16 len); static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) { int result = 0; + result = hfa384x_drvr_getconfig(hw, rid, val, sizeof(u16)); if (result == 0) *((u16 *) val) = le16_to_cpu(*((u16 *) val)); @@ -1385,6 +1386,7 @@ static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) static inline int hfa384x_drvr_setconfig16(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig(hw, rid, value, sizeof(value)); } @@ -1402,6 +1404,7 @@ static inline int hfa384x_drvr_setconfig16_async(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig_async(hw, rid, value, sizeof(value), NULL, NULL); } 1.9.1 From 5eb3de22f0760ece1e838d48c8dd9148b4331cdc Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Mon, 15 Sep 2014 17:07:24 -0400 Subject: [PATCH] staging netlogic: Fix checkpatch errors in xlr_net.c This removes the checkpatch errors related to a needed line below declaration of a struct and another about a non nessary printk message about a NULL allocated skb due to the function returning NULL to the caller of the function and the printk no longer being' used or needed by any callers. spelling mistakes Signed-off-by: Nicholas Krause xerofo...@gmail.com --- drivers/staging/netlogic/xlr_net.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c index 9bf407d..28a42831 100644 --- a/drivers/staging/netlogic/xlr_net.c +++ b/drivers/staging/netlogic/xlr_net.c @@ -142,10 +142,8 @@ static inline struct sk_buff *xlr_alloc_skb(void) /* skb-data is cache aligned */ skb = alloc_skb(XLR_RX_BUF_SIZE, GFP_ATOMIC); -if (!skb) { -pr_err(SKB allocation failed\n); why the error message was removed ? +if (!skb) return NULL; -} mac_put_skb_back_ptr(skb); return skb; } @@ -1104,6 +1102,7 @@ err_gmac: static int xlr_net_remove(struct platform_device *pdev) { struct xlr_net_priv *priv = platform_get_drvdata(pdev); + unregister_netdev(priv-ndev); mdiobus_unregister(priv-mii_bus); mdiobus_free(priv-mii_bus); -- 1.9.1 ohhh .. yeah .. and both the patch failed when i tried to apply them to next-20140916 why are we wasting our time for your patches , which are bound to have some problem thanks sudip ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies I tried these on Greg's tree of staging-next and they worked for me. Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014 08:44:27 -0400, nick said: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. I am attaching. Fail 3 words in. That's why you aren't getting a reply from the maintainers. They don't bother reading patches they know will probably *never* be correct. And yes, at least one of the two patches is incorrect. Thanks for the Help, I really do appreciate it and I do understand how much I have screwed up. My concern now is now to make it right. You don't make it right by posting more wrong patches. Now do us all a favor and *stop* being either an idiot, or a troll, or whatever your problem is. We have *zero* desire to see *any* more patches from somebody who is either unable or unwilling to post a *single* correct patch after *two months* of trying. So please do us a favor and go piss off some other open source project. THE KERNEL DOES NOT WANT YOUR BULLSHIT. pgpp7qwMl6MMy.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, Sep 16, 2014 at 6:41 PM, nick xerofo...@gmail.com wrote: On 14-09-16 09:06 AM, Sudip Mukherjee wrote: On Tue, Sep 16, 2014 at 08:44:27AM -0400, nick wrote: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. Thanks, Nick From 7bf4229fa2f9c4fcf3243bc738c74bfdc58a6594 Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Sat, 13 Sep 2014 11:53:24 -0400 Subject: [PATCH] staging wlan-ng: Add missing a blank line after declarations Fixing trivial checkpatch warnings about missing line after declarations. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- Tested by compilation only. drivers/staging/wlan-ng/hfa384x.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h index 1f2c78c..20d146b 100644 --- a/drivers/staging/wlan-ng/hfa384x.h +++ b/drivers/staging/wlan-ng/hfa384x.h @@ -1376,6 +1376,7 @@ int hfa384x_drvr_setconfig(hfa384x_t *hw, u16 rid, void *buf, u16 len); static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) { int result = 0; + result = hfa384x_drvr_getconfig(hw, rid, val, sizeof(u16)); if (result == 0) *((u16 *) val) = le16_to_cpu(*((u16 *) val)); @@ -1385,6 +1386,7 @@ static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val) static inline int hfa384x_drvr_setconfig16(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig(hw, rid, value, sizeof(value)); } @@ -1402,6 +1404,7 @@ static inline int hfa384x_drvr_setconfig16_async(hfa384x_t *hw, u16 rid, u16 val) { u16 value = cpu_to_le16(val); + return hfa384x_drvr_setconfig_async(hw, rid, value, sizeof(value), NULL, NULL); } 1.9.1 From 5eb3de22f0760ece1e838d48c8dd9148b4331cdc Mon Sep 17 00:00:00 2001 From: Nicholas Krause xerofo...@gmail.com Date: Mon, 15 Sep 2014 17:07:24 -0400 Subject: [PATCH] staging netlogic: Fix checkpatch errors in xlr_net.c This removes the checkpatch errors related to a needed line below declaration of a struct and another about a non nessary printk message about a NULL allocated skb due to the function returning NULL to the caller of the function and the printk no longer being' used or needed by any callers. spelling mistakes Signed-off-by: Nicholas Krause xerofo...@gmail.com --- drivers/staging/netlogic/xlr_net.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c index 9bf407d..28a42831 100644 --- a/drivers/staging/netlogic/xlr_net.c +++ b/drivers/staging/netlogic/xlr_net.c @@ -142,10 +142,8 @@ static inline struct sk_buff *xlr_alloc_skb(void) /* skb-data is cache aligned */ skb = alloc_skb(XLR_RX_BUF_SIZE, GFP_ATOMIC); -if (!skb) { -pr_err(SKB allocation failed\n); why the error message was removed ? +if (!skb) return NULL; -} mac_put_skb_back_ptr(skb); return skb; } @@ -1104,6 +1102,7 @@ err_gmac: static int xlr_net_remove(struct platform_device *pdev) { struct xlr_net_priv *priv = platform_get_drvdata(pdev); + unregister_netdev(priv-ndev); mdiobus_unregister(priv-mii_bus); mdiobus_free(priv-mii_bus); -- 1.9.1 ohhh .. yeah .. and both the patch failed when i tried to apply them to next-20140916 why are we wasting our time for your patches , which are bound to have some problem thanks sudip ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies I tried these on Greg's tree of staging-next and they worked for me. Nick in drivers/staging/netlogic/xlr_net.c file of next-20140916 1) in line 142 we just have an } else { (doesnot match your patch) 2) in the same file , there is only one instance of pr_err(SKB allocation failed\n); , and that is at line 208 , and that is followed by a return -ENOMEM; , but your patch is showing that there is a return NULL thanks sudip ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, Sep 16, 2014 at 10:19 AM, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 08:44:27 -0400, nick said: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. I am attaching. Fail 3 words in. That's why you aren't getting a reply from the maintainers. They don't bother reading patches they know will probably *never* be correct. And yes, at least one of the two patches is incorrect. Thanks for the Help, I really do appreciate it and I do understand how much I have screwed up. My concern now is now to make it right. You don't make it right by posting more wrong patches. Now do us all a favor and *stop* being either an idiot, or a troll, or whatever your problem is. We have *zero* desire to see *any* more patches from somebody who is either unable or unwilling to post a *single* correct patch after *two months* of trying. So please do us a favor and go piss off some other open source project. THE KERNEL DOES NOT WANT YOUR BULLSHIT. I am going to attach it again and I understand Valdis. I am going to talk to Sudip about reading and checking my patches first. Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, Sep 16, 2014 at 5:44 PM, Nick Krause xerofo...@gmail.com wrote: On Tue, Sep 16, 2014 at 10:19 AM, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 08:44:27 -0400, nick said: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. I am attaching. Fail 3 words in. That's why you aren't getting a reply from the maintainers. They don't bother reading patches they know will probably *never* be correct. And yes, at least one of the two patches is incorrect. Thanks for the Help, I really do appreciate it and I do understand how much I have screwed up. My concern now is now to make it right. You don't make it right by posting more wrong patches. Now do us all a favor and *stop* being either an idiot, or a troll, or whatever your problem is. We have *zero* desire to see *any* more patches from somebody who is either unable or unwilling to post a *single* correct patch after *two months* of trying. So please do us a favor and go piss off some other open source project. THE KERNEL DOES NOT WANT YOUR BULLSHIT. I am going to attach it again and I understand Valdis. I am going to talk to Sudip about reading and checking my patches first. Rik, probably it's time to ban Nick also from this ml. -- Carlo Caione ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
Nick, Use this guide: http://kernelnewbies.org/OPWfirstpatch It's a tutorial that shows you how to setup and send patches. And memorize these references: http://lxr.free-electrons.com/source/Documentation/SubmitChecklist http://lxr.free-electrons.com/source/Documentation/development-process/ Lidza On Tue, Sep 16, 2014 at 11:50 AM, Carlo Caione ca...@caione.org wrote: On Tue, Sep 16, 2014 at 5:44 PM, Nick Krause xerofo...@gmail.com wrote: On Tue, Sep 16, 2014 at 10:19 AM, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 08:44:27 -0400, nick said: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. I am attaching. Fail 3 words in. That's why you aren't getting a reply from the maintainers. They don't bother reading patches they know will probably *never* be correct. And yes, at least one of the two patches is incorrect. Thanks for the Help, I really do appreciate it and I do understand how much I have screwed up. My concern now is now to make it right. You don't make it right by posting more wrong patches. Now do us all a favor and *stop* being either an idiot, or a troll, or whatever your problem is. We have *zero* desire to see *any* more patches from somebody who is either unable or unwilling to post a *single* correct patch after *two months* of trying. So please do us a favor and go piss off some other open source project. THE KERNEL DOES NOT WANT YOUR BULLSHIT. I am going to attach it again and I understand Valdis. I am going to talk to Sudip about reading and checking my patches first. Rik, probably it's time to ban Nick also from this ml. -- Carlo Caione ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014 11:57:35 -0400, Lidza Louina said: Use this guide: http://kernelnewbies.org/OPWfirstpatch It's a tutorial that shows you how to setup and send patches. Don't bother. He's been pointed at that at least once a week for the past two months, and everybody from me to Greg KH to Ted T'so and at least a half dozen other experienced kernel people have told him *repeatedly* what to do and not do, and he has *yet* to post a correct patch - and he keeps making the same mistakes that he was told not to. For instance, I didn't even *bother* looking at his last two patches, because they were attachments, even *after* Robert PJ Day told him *yesterday* to not do that. pgp35oNCc5ASc.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, Sep 16, 2014 at 11:44:59AM -0400, Nick Krause wrote: On Tue, Sep 16, 2014 at 10:19 AM, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 08:44:27 -0400, nick said: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. I am attaching. Fail 3 words in. [snip] I am going to attach it again and I understand Valdis. I am going to talk to Sudip about reading and checking my patches first. The email that you are replying to repeats the statement that attaching the patches is the wrong thing to be doing. And yet, in the reply, you talk about attaching patches. Can you please explain what's going on here? Did you forget about it while you were writing the three line response? Did you simply ignore the words? Did you assume that it didn't apply to you? Did you merely think it wasn't important? Do you have short-term memory problems? Is there something else I don't know about? I understand that you're a student (or will shortly be one), and so far you're showing *major* learning difficulties, without any apparent self-identification of the fact. If you do actually have some kind of learning difficulty, you're going to have to take charge of it yourself, seek professional advice, use that to work out how you can learn, and start doing it, because if you behave like this in any institution of higher learning, you're going to have big problems, probably very quickly. You've spent two months, with *lots* of explicit recommendations on how you should be modifying your behaviour (i.e. learning), and so far you've utterly failed to do so. It's up to you to seek help in identifying the difficulties you have in learning, and to use the results of that to work out how you can do so. This kernel development environment is *absolutely* the wrong one for you to be doing that exercise in. I doubt that there are many (if any) people on this mailing list who have the knowledge or skills to help you. I am certain that if there are any, none of them would be willing to start the process via email. Hugo. -- === Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk === PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk --- Big data doesn't just mean increasing the font size --- signature.asc Description: Digital signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 11:57:35 -0400, Lidza Louina said: Use this guide: http://kernelnewbies.org/OPWfirstpatch It's a tutorial that shows you how to setup and send patches. Don't bother. He's been pointed at that at least once a week for the past two months, and everybody from me to Greg KH to Ted T'so and at least a half dozen other experienced kernel people have told him *repeatedly* what to do and not do, and he has *yet* to post a correct patch - and he keeps making the same mistakes that he was told not to. For instance, I didn't even *bother* looking at his last two patches, because they were attachments, even *after* Robert PJ Day told him *yesterday* to not do that. i posted three simple pieces of advice yesterday: * proper grammar, at the very least in the subject line * make sure the commit description matches the patch * no attachments nick promptly violated all of the above. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014, Lidza Louina wrote: Nick, Use this guide: http://kernelnewbies.org/OPWfirstpatch It's a tutorial that shows you how to setup and send patches. And memorize these references: http://lxr.free-electrons.com/source/Documentation/SubmitChecklist http://lxr.free-electrons.com/source/Documentation/development-process/ Lidza i'm going to back up carlo ... time to ban nick as i suggested a while back. giving nick more references is not going to help -- he is simply unteachable. in a nutshell, nick brings no value to this mailing list. quite the opposite -- he devalues it significantly by wasting bandwidth and just plain pissing people off. all by himself, nick makes this mailing list a worse place. if nick showed any sign of progress whatsoever, that would be different. but he doesn't. so, please, pull the plug on him. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On 9/16/14, Nick Krause xerofo...@gmail.com wrote: On Tue, Sep 16, 2014 at 10:19 AM, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 08:44:27 -0400, nick said: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. I am attaching. Fail 3 words in. That's why you aren't getting a reply from the maintainers. They don't bother reading patches they know will probably *never* be correct. And yes, at least one of the two patches is incorrect. Thanks for the Help, I really do appreciate it and I do understand how much I have screwed up. My concern now is now to make it right. You don't make it right by posting more wrong patches. Now do us all a favor and *stop* being either an idiot, or a troll, or whatever your problem is. We have *zero* desire to see *any* more patches from somebody who is either unable or unwilling to post a *single* correct patch after *two months* of trying. So please do us a favor and go piss off some other open source project. THE KERNEL DOES NOT WANT YOUR BULLSHIT. I am going to attach it again and I understand Valdis. I am going to talk to Sudip about reading and checking my patches first. Nick plzzz .. NO ... today i checked your patches just to take a break from what i was doing.. and besides , i am also a newbie , i also make mistakes when submitting patches, but i try to learn from my mistakes and see to it that they are never repeated. apart from the numerous advises you got from the LKML , i think just yesterday Robert P. J. Day has pointed out something, day before yesterday Valdis , before that ( sorry i lost track) ... and you have not cared to listen to any of that .. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, Sep 16, 2014 at 12:57 PM, Sudip Mukherjee sudipm.mukher...@gmail.com wrote: On 9/16/14, Nick Krause xerofo...@gmail.com wrote: On Tue, Sep 16, 2014 at 10:19 AM, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 08:44:27 -0400, nick said: I am attaching two check patch patches I wrote in the last few days as I am unable to get a reply from the maintainers. Would someone please send them off for me. I am attaching. Fail 3 words in. That's why you aren't getting a reply from the maintainers. They don't bother reading patches they know will probably *never* be correct. And yes, at least one of the two patches is incorrect. Thanks for the Help, I really do appreciate it and I do understand how much I have screwed up. My concern now is now to make it right. You don't make it right by posting more wrong patches. Now do us all a favor and *stop* being either an idiot, or a troll, or whatever your problem is. We have *zero* desire to see *any* more patches from somebody who is either unable or unwilling to post a *single* correct patch after *two months* of trying. So please do us a favor and go piss off some other open source project. THE KERNEL DOES NOT WANT YOUR BULLSHIT. I am going to attach it again and I understand Valdis. I am going to talk to Sudip about reading and checking my patches first. Nick plzzz .. NO ... today i checked your patches just to take a break from what i was doing.. and besides , i am also a newbie , i also make mistakes when submitting patches, but i try to learn from my mistakes and see to it that they are never repeated. apart from the numerous advises you got from the LKML , i think just yesterday Robert P. J. Day has pointed out something, day before yesterday Valdis , before that ( sorry i lost track) ... and you have not cared to listen to any of that .. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies I am going to resend my patch and see if it's good and if not I am going to leave for a while. Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014, Nick Krause wrote: I am going to resend my patch and see if it's good ... because, of course, that's the correct order of events -- send the patch, *then* validate its correctness. ban. for the love of mutt, please, ban. there is no excuse for this anymore. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/16/2014 01:12 PM, Nick Krause wrote: I am going to resend my patch and see if it's good and if not I am going to leave for a while. You may want to consider fixing them, by applying the advice provided by many people, before resending them. - -- All rights reversed. -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJUGHMmAAoJEM553pKExN6Dvw4IAKi2no5OpH9TUgcCd5zRHTsb oqFsZ2xR21bgnIyfM93V+h9QtG+Fuc+WOScCL1Ofk4OWVXWV6gIgHHSGBOb0cEBf zGTxhMyRdPUl7WHAn7KHWAEqhCGQlIp0QyyRiXjnNLmSmGYt/9yv3TrvmZE0xuJw /jrq+H93AxTXXVzIjfpnziXMD5w3/Zt9+FPlyuNjyVh2Hnlf2CbwfSqsgsJETZOx ML+CNsl/2CAXkJ1++CUQ94iC4plAPekVrpXAea4xqWqargD8Hcm0jqWuD9vns0ZA VDJU669ocBlZRVCdZI6axGm4JAdtceQLLMMVZtzre8IAELB9JPiUC/C6g7e60Cg= =oeV3 -END PGP SIGNATURE- ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014 13:12:32 -0400, Nick Krause said: I am going to resend my patch and see if it's good Why bother? We already *know* it won't be. You could amaze and astound us all by seeing if it's good *before* you hit send. But I'm not holding my breath. and if not I am going to leave for a while. Actions. Not words. pgpPFmcc0SEED.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014 13:28:06 -0400, Rik van Riel said: -BEGIN PGP SIGNED MESSAGE- On 09/16/2014 01:12 PM, Nick Krause wrote: I am going to resend my patch and see if it's good and if not I am going to leave for a while. You may want to consider fixing them, by applying the advice provided by many people, before resending them. That's just crazy talk. pgptnWJkjpmHX.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, Sep 16, 2014 at 1:39 PM, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 13:28:06 -0400, Rik van Riel said: -BEGIN PGP SIGNED MESSAGE- On 09/16/2014 01:12 PM, Nick Krause wrote: I am going to resend my patch and see if it's good and if not I am going to leave for a while. You may want to consider fixing them, by applying the advice provided by many people, before resending them. That's just crazy talk. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies I am fixing them first and them sending them out. I am going to listen now and only do them correctly. Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/16/2014 02:36 PM, Kristofer Hallin wrote: On Tue, Sep 16, 2014 at 8:29 PM, Nick Krause xerofo...@gmail.com wrote: On Tue, Sep 16, 2014 at 1:39 PM, valdis.kletni...@vt.edu wrote: On Tue, 16 Sep 2014 13:28:06 -0400, Rik van Riel said: -BEGIN PGP SIGNED MESSAGE- On 09/16/2014 01:12 PM, Nick Krause wrote: I am going to resend my patch and see if it's good and if not I am going to leave for a while. You may want to consider fixing them, by applying the advice provided by many people, before resending them. That's just crazy talk. I am fixing them first and them sending them out. I am going to listen now and only do them correctly. Nick No, you won't. Nick, this is a chance for you to prove the nay-sayers wrong (or right). When re-posting your patch, you can include information on what tips you got on how to improve the patch, and how you applied that information in the new version of the patch. This is a good idea for anybody resubmitting a changed patch, because it tells other reviewers what changes were made, why, and who pointed out the idea for the change. - -- All rights reversed. -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJUGIUeAAoJEM553pKExN6DEt8H/36FILPmG4wOK3TZFpQEVJHv Lxq4DLq7GBUS37PhL/wjdYnLD4RJmOhpGk+FlZBhJ0StnCvS38Su7wwyZTzgCPIS ir3EYOKhiFgEEuoB4iVaDc2FNv9jKKRVy/QK3jwkdt5oTBvr/I6YZzj2XAFJaUp9 0/RvdSNZU6kcvhTkd5xdDaaX1iOpxrggccRl+Pv2GBapeIEKvO6zk9U2GuLw+r6q OLfJ5BElpEReoqFJNUXxzT+UyTmMel21REdfyu9UxAEONrQ2A3UHx/7upy71YpyR a/IADIuBSXTdb31b+uuoFPfSUp4cs7WMyIDK2/tlL8wF28GLoL39X2xhWE3gkUU= =7mL1 -END PGP SIGNATURE- ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
I am fixing them first and them sending them out. I am going to listen now and only do them correctly. Nick, you're very close to being banned from kernelnewbies too, so please give some thought to other people's advise or else this might be your last email to kernelnewbies. As a first step, DON'T send patches as attachments, send them INLINE in PLAIN TEXT. You need to demonstrate an understanding of this basic instruction or else nobody will be able to help you. And frankly, you need to stop fixing anything in the kernel till you get some level of proficiency in basic programming and working collaboratively in opensource projects. Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: Checkpatch Patches
On Tue, 16 Sep 2014, Rik van Riel wrote: Nick, this is a chance for you to prove the nay-sayers wrong (or right). When re-posting your patch, you can include information on what tips you got on how to improve the patch, and how you applied that information in the new version of the patch. This is a good idea for anybody resubmitting a changed patch, because it tells other reviewers what changes were made, why, and who pointed out the idea for the change. my god, rik, you're adorable. :-) i predict that, despite what i've posted *twice*, nick will describe as an error what is simply a checkpatch coding style *warning*. let's watch ... rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
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 :) pgpmlpJ3Qbd8_.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Tue, Sep 16, 2014 at 7:25 PM, Valdis Kletnieks valdis.kletni...@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=staging-nextqt=authorq=gdonald 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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
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, 1 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) 0x,0x,0x,0x,0x,0x,0x,0x,/* 0x00-0x07 */ ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 18) 0x,0x,0x,0x,0x,0x,0x,0x,/* 0x08-0x0F */ ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 19) 0x,0x,0x,0x,0x,0x,0x,0x,/* 0x10-0x17 */ ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 20) 0x,0x,0x,0x,0x,0x,0x,0x,/* 0x18-0x1F */ ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 21) 0x,0x,0x,0x,0x,0x,0x,0x,/* 0x20-0x27 */ ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 22) 0x,0x,0x,0x,0x,0x,0x,0x,/* 0x28-0x2F */ ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 23) 0x,0x,0x,0x,0x,0x,0x,0x,/* 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) pgp0qMqErYDWh.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Tue, Sep 16, 2014 at 11:42:18PM -0400, valdis.kletni...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: A quick guide to why stand-alone checkpatch patches suck...
On Wed, Sep 17, 2014 at 10:26 AM, Greg KH g...@kroah.com wrote: On Tue, Sep 16, 2014 at 11:42:18PM -0400, valdis.kletni...@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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies