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