Messages in this thread |  | | | Date | Fri, 07 Jan 2011 08:05:09 -0500 | | From | William Allen Simpson <> | | Subject | Re: [058/152] tcp: protect sysctl_tcp_cookie_size reads |
| |
On 1/7/11 3:30 AM, Eric Dumazet wrote: > Are you telling us somebody else added bug to your code ? > This is not the case, obviously. > As I noted, "There's been quite a few changes" -- one of which was a major change to sysctls. As one of the reviewers of this code, you didn't mention this ACCESS_ONCE() function, so I'm assuming it didn't exist at that time.
> After bug fix and cleanup code
As I noted, "most of the patch has nothing to do with the purported fix." That makes the patch hard to read and evaluate.
Moreover, I remember being castigated (by you and others) for combining bug fixes with cleanup code. And I'm not sure this counts as "cleanup" code.
> looks good,
As the saying goes, beauty is in the eye of the beholder. Trading a blank line for a trailing brace is neither here nor there. Screen space used remains the same.
> and even checkpatch.pl is > fine with it. No need for useless brackets around "return XXX;"
My instructors would have flunked me for not including braces around multi-line sequences; it was one of the great no-no's of the '70s. Perhaps that's not the case anymore with modern colorful visual syntax checkers?
> If I remember well, I did the cleanup so that my patch could not trigger > checkpatch.pl errors/warnings. Not that I am a particular checkpatch > fan, but I know some people are. > As a relative Linux kernel newbie, I scrupulously followed patch instructions. Those instructions mandated running checkpatch.
If there are now "checkpatch.pl errors/warnings" on that code, checkpatch must have changed. At the time, all multi-line sequences were required to be enclosed in braces.
I still think that's a better idea, looks better, and makes maintenance easier in the long term. YMMV.
> By the way, you were CCed when I sent one month ago the mail to > David/netdev. And no reaction from you at that time. > Amazingly enough, my life is not centered around Linux on a day-to-day basis. Young folks like you are trying to make a name for yourselves in the Linux world -- apparently, by being exceptionally abrasive.
Your helpful comments are/were appreciated.
|  |