Daiki Ueno <u...@gnu.org> writes:
> Thank you. The option (3) sounds like a great idea as it only need one
> more function to be added for streaming. I tried to implement it as the
> attached patch.
Thanks. Interface and tests looks very reasonable to me. Comments on the
implementatino below.
Regards,
/Niels
> +void
> +sha3_256_shake_output(struct sha3_256_ctx *ctx,
> + size_t length,
> + uint8_t *digest)
> +{
> + unsigned offset;
> + unsigned mask = UINT_MAX >> 1;
I think I'd name the local variable "index" rather than "offset", to
match the state variable. And I think it would make sense with a define
for the high bit, something like
#define INDEX_HIGH_BIT (~((UINT_MAX) >> 1))
(one could also use something like ~0U instead of UINT_MAX, but UINT_MAX
may be more readable).
> + /* We use the leftmost bit as a flag to indicate SHAKE is initialized. */
> + if (ctx->index & ~mask)
> + offset = ctx->index & mask;
The value of offset here is in the range 0 < offset <=
SHA3_256_BLOCK_SIZE, right? One could use a representation where
offset = ~ctx->index;
instead of bitwise operations. One would still need the condition if
(ctx->index & INDEX_HIGH_BIT), but that would typically be compiled to
the same as if ((signed int) ctx->index < 0).
I think it would also make sense with an
assert (ctx->index < SHA3_256_BLOCK_SIZE);
in the start of sha3_256_update, which will trigger if the update
function is called after the output function, with no init in between.
> + else
> + {
> + _sha3_pad_shake (&ctx->state, SHA3_256_BLOCK_SIZE, ctx->block,
> ctx->index);
> + /* Point at the end of block to trigger fill in of the buffer. */
> + offset = sizeof (ctx->block);
I think this block deserves a comment that this is the first call to
sha3_256_shake_output. For the block size, I think it would be nice to
consitently use one of SHA3_256_BLOCK_SIZE and sizeof (ctx->block).
> + }
> +
> + for (;;)
> + {
> + /* Write remaining data from the buffer. */
> + if (offset < sizeof (ctx->block))
> + {
> + unsigned remaining;
> +
> + remaining = MIN(length, sizeof (ctx->block) - offset);
> + memcpy (digest, &ctx->block[offset], remaining);
> + digest += remaining;
> + offset += remaining;
I think handling of the leftover can be moved before the loop, and
simplified as
unsigned left = sizeof(ctx->block) - offset;
if (length <= left)
{
memcpy (digest, ctx->block + offset, length);
ctx->index = (offset + length) | INDEX_HIGH_BIT;
return;
}
memcpy (digest, ctx->block + offset, left);
digest += left;
length -= left;
followed by a loop
for (; length >= SHA3_256_BLOCK_SIZE;
length -= SHA3_256_BLOCK_SIZE, digest += SHA3_256_BLOCK_SIZE)
{
... output a full block ...
}
if (length > 0)
{
... do final partial block ...
ctx->index = length | INDEX_HIGH_BIT;
}
else
ctx->index = SHA3_256_BLOCK_SIZE | INDEX_HIGH_BIT;
--
Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se
To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
