lkml.org 
[lkml]   [2011]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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.


    \
     
     \ /
      Last update: 2011-01-07 14:07    [W:0.022 / U:30.412 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site