Date:	Mon, 21 Jan 2013 05:20:27 -0500 (EST)
From:	Tom St Denis <tstdenis@...iptictech.com>
To:	David Dillow <dave@...dillows.org>
Cc:	Borislav Petkov <bp@...en8.de>, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>
Subject: Re: IPsec AH use of ahash



----- Original Message -----
> From: "David Dillow" <dave@...dillows.org>
> To: "Tom St Denis" <tstdenis@...iptictech.com>
> Cc: "Borislav Petkov" <bp@...en8.de>, linux-kernel@...r.kernel.org, netdev@...r.kernel.org, "Steven Rostedt"
> <rostedt@...dmis.org>
> Sent: Monday, 21 January, 2013 4:18:01 AM
> Subject: Re: IPsec AH use of ahash
> 
> On Sun, 2013-01-20 at 19:40 -0500, Tom St Denis wrote:
> > The problem is for me to add the ()'s is a no brainer.
> 
> It was more than just the ()'s. You completely botched the
> indentation
> in the parts of code you didn't copy from xcbc.c, among other issues.

Leaving coding indentation aside [I assume you mean the use of braces because my tab stops should match given the patchset for the other files was minimal] that's not really a good use of time here.
 
> > For me to re-write complete statements for coding style reasons
> > means
> > I have to actually go out and test it again.
> 
> Well, for some of the code you submitted, that would be the first
> testing it got, even compile testing AFAICT. I don't doubt that you
> tested against your hardware, but it is obvious that you didn't run
> the
> algorithm tests you added, or even tried to compile them. You were
> missing a semicolon after your test data, and you were also missing a
> "\x" in there, so your test data was wrong.

The missing semi-colon is in fact missing and for that I apologize.  This btw is the first legitimate gripe with the code thus far.

Though I must say this code has "compiled" in 3.6, 3.6.7, 3.7, and 3.8 without error so it's not surprising it wasn't caught.  After having consulted the "documentation" I never really did figure out how to get testmgr to run so I took a bit of leap of faith there that everything was fine.  Not really an excuse but it is at least an observable hole in the build documentation.

I couldn't spot the missing \x but I don't doubt you.  That format for vector data is so backwards that it's easy to make that mistake.  After having worked on crypto code for 10+ years I've never seen anyone use that format and I've seen stupid formats before (NIST CAVP anyone?).

Again all valid critiques of the code and valid reasons to do a re-spin.  I apologize for the oversight there.  Having "used the source" I found the build symbol required to activate the test manager so I should be good to go now.

> I know this, because I just spent the five minutes required to fix up
> the checkpatch warnings to prove to myself that you could have done
> it
> faster than sending one considered message to the list about how much
> we
> suck. Of course, that assumes you are trying to have a discussion
> instead of trolling; it's much easier to spew bile than to think
> before
> you post.

This wasn't about how you all suck.  Quite the contrary it was about how good people are missing perspective.  You all started this by harping on the coding style "problems" but as a new developer to the kernel (well at least a new submitter...) I used the source as a guide so that I wouldn't deviate from what I perceived as the norm.  I was legitimately frustrated because performing a re-spin solely to work past coding style problems is not something a business case could be made for.  I'd like to contribute properly but at the same time you can't throw red flags on the play each time someone plays the game like you do.

At least the testmgr errors are something I can work on whenever without setting up a lab so likely that'll be something I can tackle today actually.

Tom

--
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

Powered by blists - more mailing lists