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

From: William Allen Simpson
Date: Fri Jan 07 2011 - 08:05:25 EST

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 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 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 " 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
Please read the FAQ at