Re: [058/152] tcp: protect sysctl_tcp_cookie_size reads

[William Allen Simpson]

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/