On Fri, 5 Sep 2014, Behan Webster wrote: > On 09/05/14 17:18, Thomas Gleixner wrote: > > > Signed-off-by: Behan Webster <beh...@converseincode.com> > > > Signed-off-by: Mark Charlebois <charl...@gmail.com> > > > Signed-off-by: Jan-Simon Möller <dl...@gmx.de> > > This SOB chain is completely ass backwards. See Documentation/... > "The Signed-off-by: tag indicates that the signer was involved in the > development of the patch, or that he/she was in the patch's delivery path." > > All three of us were involved. Does that not satisfy this rule?
No. Read #12 The sign-off is a simple line at the end of the explanation for the patch, which certifies that you wrote it or otherwise have the right to pass it on as an open-source patch. So the above chain says: Written-by: Behan Passed-on-by: Mark Passed-on-by: Jan That would be correct if you sent the patch to Mark, Mark sent it to Jan and Jan finally submitted it to LKML. > > > - struct { > > > - struct shash_desc shash; > > > - char ctx[crypto_shash_descsize(tfm)]; > > > - } desc; > > > + char desc[sizeof(struct shash_desc) + > > > + crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR; > > > + struct shash_desc *shash = (struct shash_desc *)desc; > > That anon struct should have never happened in the first place. > Sadly this is a design pattern used in many places through out the kernel, and > appears to be fundamental to the crypto system. I was advised *not* to change > it, so we haven't. > > I agree that it's not a good practice. > > > Not > > your problem, but you are not making it any better. You replace open > > coded crap with even more unreadable crap. > > > > Whats wrong with > > > > SHASH_DESC_ON_STACK(shash, tfm); > Nothing is wrong with that. I would have actually preferred that. But it would > have fundamentally changed a lot more code. Errm. Why is #define SHASH_DESC_ON_STACK(shash, tfm) \ char __shash[sizeof(.....)]; \ struct shash_desc *shash = (struct shash_desc *) __shash requiring more fundamental than open coding the same thing a gazillion times. You still need to change ALL usage sides of the anon struct. So in fact you could avoid the whole code change by making it SHASH_DESC_ON_STACK(desc, tfm); and do the anon struct or a proper struct magic in the macro. > I'm not sure making fundamental changes to the crypto code in order to make > "my favourite compiler happy" is really a better plan. I prefer smaller more > measured steps. What's fundamental about a change which produces the same code but hides all the easy to get wrong mess in a single place? > > or something along that line and hide all the stack allocation, type > > conversion crap and make my favourite compiler happy in a single > > place? > At this point it is less about making a particular compiler happy, and more > about making the code at least C99 compliant which enables a different > compiler so that more people can use it to make more fundamental changes like > you are suggesting. Well, just blindly making it compliant is one thing. But if we do that we really should make it better and using a macro for this is definitely an improvement which is worthwhile to do. Thanks, tglx