* Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Jul 01, 2016 at 09:55:59AM +0200, Ingo Molnar wrote:
> >
> > Plus:
> >
> > > > > /* Compute how many bytes to copy from user buffer into
> > > > > * extra block
> > > > > */
> >
> > please use the customary (multi-line) comment style:
>
> This is the customary comment style of the networking stack and
> the crypto API. So please don't change it.
Guys, do you even read your own code??
That 'standard' is not being enforced consistently at all. Even in this very
series there's an example of that weird comment not being followed:
+++ b/arch/x86/crypto/sha1-mb/sha1_mb.c
@@ -304,7 +304,7 @@ static struct sha1_hash_ctx *sha1_ctx_mgr_submit(struct sha1_ctx_mgr *mgr,
/*
* Compute how many bytes to copy from user buffer into
* extra block
See how this comment block uses the standard coding style, while the next patch
has this weird coding style:
- if ((ctx->partial_block_buffer_length) | (len < SHA256_BLOCK_SIZE)) {
+ if ((ctx->partial_block_buffer_length) || (len < SHA256_BLOCK_SIZE)) {
/* Compute how many bytes to copy from user buffer into
* extra block
*/
The networking code's "exceptionalism" regarding the standard comment style is
super distracting and in this particular example it resulted in:
- inconsistent comment styles next to each other,
- the questionable '|' pattern hiding right next to:
- pointless parantheses around the (ctx->partial_block_buffer_length),
- which field name is also a misnomer.
So anyone doing security review of that weird '|' pattern first has to figure out
whether the 4 ugly code patterns amount to a security problem or not...
One thing that is more harmful that any of the coding styles: the inconsistent
coding style used by this code.
Btw., as a historic reference, there is nothing sacred about the 'networking
comments coding style': I was there (way too many years ago) when that comment
style was introduced by Alan Cox's first TCP/IP code drop, and it was little more
than just a random inconsistency that people are now treating as gospel...
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
