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

William Allen Simpson
2011-01-07T13:05:19Z

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.