Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1435261

Re: [patch] crypto: sha256-mb - cleanup a || vs | typo

Show all headers | View raw

* Herbert Xu <herbert@gondor.apana.org.au> 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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[patch] crypto: sha256-mb - cleanup a || vs | typo Dan Carpenter <dan.carpenter@oracle.com> - 2016-06-29 16:50 +0200
  Re: [patch] crypto: sha256-mb - cleanup a || vs | typo "H. Peter Anvin" <hpa@zytor.com> - 2016-06-29 19:10 +0200
    Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Dan Carpenter <dan.carpenter@oracle.com> - 2016-06-30 10:00 +0200
      Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Joe Perches <joe@perches.com> - 2016-06-30 13:20 +0200
        Re: [patch] crypto: sha256-mb - cleanup a || vs | typo walter harms <wharms@bfs.de> - 2016-06-30 13:50 +0200
          Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Dan Carpenter <dan.carpenter@oracle.com> - 2016-06-30 14:40 +0200
    Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Tim Chen <tim.c.chen@linux.intel.com> - 2016-06-30 22:50 +0200
      Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Dan Carpenter <dan.carpenter@oracle.com> - 2016-07-01 00:20 +0200
      Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Ingo Molnar <mingo@kernel.org> - 2016-07-01 10:00 +0200
        Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Herbert Xu <herbert@gondor.apana.org.au> - 2016-07-01 11:30 +0200
          Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Ingo Molnar <mingo@kernel.org> - 2016-07-01 13:20 +0200
            Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Tim Chen <tim.c.chen@linux.intel.com> - 2016-07-08 18:30 +0200
              Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Herbert Xu <herbert@gondor.apana.org.au> - 2016-07-08 18:50 +0200
                Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Linus Torvalds <torvalds@linux-foundation.org> - 2016-07-08 19:20 +0200
                Re: [patch] crypto: sha256-mb - cleanup a || vs | typo Tim Chen <tim.c.chen@linux.intel.com> - 2016-07-08 19:20 +0200

csiph-web