netdev - Re: [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4AF97C10.9020208@gmail.com>
Date:	Tue, 10 Nov 2009 09:43:28 -0500
From:	William Allen Simpson <william.allen.simpson@...il.com>
To:	Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>
CC:	Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size,
 socket option TCP_COOKIE_TRANSACTIONS, functions

Ilpo Järvinen wrote:
> I guess most of the cookie stuff have nothing to do with the next, 
> please make them separate:
> 
>> Redefine two TCP header functions to accept TCP header pointer.
>> When subtracting, return signed int to allow error checking.
> 
> Convert the users here, not in the fifth patch to avoid noise in the large 
> fifth patch.
> 
These comments are directly contrary to advice back on the 2nd or 3rd
round, where I'd both converted and removed the old functions here, before
going on to use them everywhere.  I was told to *only* define the new ones
here, use them in later patches as they applied, and then remove the old
functions in a final cleanup patch.

When I'm done (apparently very far in the distant future), there will be
approximately 100 (maybe 150 or more) uses.

> And, I read more that fifth patch... seriously, please consider now to 
> apply _all_ the coding style changes that have been brought to your 
> attention by multiple people (you should know them now but for some reason 
> again you choose to resent without complying -- I guess on purpose) into 
> all upcoming patches you submit.
> 
Pardon me, but I'm having difficulty finding a substantive comment.  Are
you attempting humor or sarcasm?  If so, it's not readily apparent.  It
seems more of an /ad hominem/ attack to me.

There were many painstaking hours of coding style changes, every single
one of them had to be individually examined side by side for correctness
after I ran a series of regexp over the patches -- in 148 blocks (I just
checked) of diff between the patch diffs, comprising eye-glazing trivial
changes (although I used BBEdit's very nice GUI utility for the
side-by-side comparison).

If you don't like something else, please be specific (as others did
privately).  There may be places the regexp missed.

But more importantly, I'd prefer actual analysis.  Is there a better
Linux function to use for something (like RCU that Eric recommended)?

This is the *easy* patch series, based on code previously reviewed.  The
next series are much more conceptually difficult.  Maybe that's the
problem, this is too easy, and so the only things people find to comment
about are trivia.

Please, more thoughtful code review, less interpersonal sniping.

TIA.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html