[PATCH] sha3sum: New applet

Mike Frysinger vapier at gentoo.org
Thu Jan 3 22:53:04 UTC 2013


On Thursday 03 January 2013 15:04:54 Lauri Kasanen wrote:
> From bbe1511447d6e120770d9d72e717cc18986535e5 Mon Sep 17 00:00:00 2001
> From: Lauri Kasanen <curaga at operamail.com>
> Date: Thu, 3 Jan 2013 21:10:01 +0200
> Subject: [PATCH] sha3sum: New applet

can't you `git send-email` ?  attaching patches encoded in base64 makes it a 
pain to review.

> --- a/coreutils/md5_sha1_sum.c
> +++ b/coreutils/md5_sha1_sum.c
>
>  	HASH_SHA1 = '1',
>  	HASH_SHA256 = '2',
>  	HASH_SHA512 = '5',
> +	HASH_SHA3 = '3',
>  };

keep sorted

> +/*
> +The Keccak sponge function, designed by Guido Bertoni, Joan Daemen,
> +Michael Peeters and Gilles Van Assche. For more information, feedback or
> +questions, please refer to our website: http://keccak.noekeon.org/

please clean up the comment block
/* this is a comment
 * block blah blah
 */

> +#if BB_LITTLE_ENDIAN
> +	while (--laneCount >= 0) {
> +		state[laneCount] ^= in[laneCount];
> +	}

we do not want "#if" blocks if possible.  use:
	if (BB_LITTLE_ENDIAN) {
		...
	} else {
		...
	}

> +	while (--laneCount >= 0) {
> +		for (x = 0; x < sizeof(uint64_t); ++x) {
> +			temp <<= 8;
> +			temp |= ((char *)&in[laneCount])[x];
> +		}
> +		state[laneCount] = temp;

can't you merge these two code paths:
	while (--laneCount >= 0) {
		state[laneCount] ^= SWAP_LE64(in[laneCount]);

seems like you do an xor in the LE case but not the BE case ?  i'm guessing 
that's a bug.

> +#define	round laneCount

why ?  why not adjust the code or the local variables instead ?

> +	for ( /* empty */ ; (databitlen >= 8) && (state->bitsInQueue != 0);
> +	for ( /* */ ; databitlen >= cKeccakR;
> +	for ( /* empty */ ; databitlen >= 8;

i would just delete those first /* */ comments.  they should at least be 
consistent if you really want to keep them.

> +	// Padding

we use /**/ comments, not //

> +#if BB_LITTLE_ENDIAN
> +	memcpy(hashval, state->state, hashbytelen);
> +#else
> +	for (offset = 0; offset < hashbytelen; offset += sizeof(uint64_t)) {
> +		uint8_t j;
> +
> +		for (j = 0; j < sizeof(uint64_t); ++j) {
> +			hashval[offset + j] =
> +			    state->state[offset + (sizeof(uint64_t) - 1) - j];
> +		}
> +	}
> +#endif

same comment here about "if (BB_LITTLE_ENDIAN)" vs "#if BB_LITTLE_ENDIAN)"
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20130103/1f8a3b03/attachment.asc>


More information about the busybox mailing list