linux-kernel - Re: [PATCH] add slice by 8 algorithm to crc32.c

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] [day] [month] [year] [list]

Date:	Mon, 1 Aug 2011 12:39:14 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	"Bob Pearson" <rpearson@...temfabricworks.com>
Cc:	"'frank zago'" <fzago@...temfabricworks.com>,
	<linux-kernel@...r.kernel.org>,
	"'Roland Dreier'" <roland@...nel.org>
Subject: Re: [PATCH] add slice by 8 algorithm to crc32.c

On Thu, 28 Jul 2011 20:47:36 -0500
"Bob Pearson" <rpearson@...temfabricworks.com> wrote:

> > 
> > > +
> > > +	init_bytes = (uintptr_t)p32 - (uintptr_t)p8;
> > 
> > Please take a look at the types of init_bytes and end_bytes.
> > ptrdiff_t, size_t and uint would all eb more appropriate than `int'.
> 
> Is there a performance difference on 32 bit machines from using a long to
> hold something that is in the range [0,7]? Which of these would you
> recommend?

I'm not aware of 64-bit being any faster than 32-bit on any machine. 
But I'm not aware of much.

Making this a signed quantity was inappropriate.  unsigned int, perhaps.

IMO, unsigned should have been the default in C - negative numbers are
relatively rare.  Whatever.

> > > +	p32 = (u32 *)(((uintptr_t)p8 + 3) & ~3);
> > 
> > ALIGN()?
> 
> Sure.
> 
> > 
> > > +	init_bytes = (uintptr_t)p32 - (uintptr_t)p8;
> > > +	if (init_bytes > len)
> > > +		init_bytes = len;
> > 
> > max()?
> 
> Maybe. First line is the main thought and the if handles the exception where
> the unrolled loop doesn't have a middle. Happy to oblige though. How about
> an unlikely in the if?

Don't care much ;) It's just that the reader looks at those two lines,
absorbs them then says "ah, it's taking the minimum".  If it had just
used "min()" then review and reading are more efficient, and reliable.  Plus min()
has typechecking features which can catch slipups.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/