dbarysh...@gmail.com writes:
> From: Dmitry Eremin-Solenikov <dbarysh...@gmail.com>
>
> Move Galois polynomial shifts to block-internal.h, simplifying common
> code. GCM is left unconverted for now, this will be fixed later.
Thanks for cleaning this up! Some comments below.
> --- a/block-internal.h
> +++ b/block-internal.h
> @@ -90,4 +90,80 @@ block8_xor_bytes (union nettle_block8 *r,
> memxor3 (r->b, x->b, bytes, 8);
> }
>
> +#define LSHIFT_WORD(x) ((((x) & 0x7f7f7f7f7f7f7f7f) << 1) | \
> + (((x) & 0x8080808080808080) >> 15))
> +#define RSHIFT_WORD(x) ((((x) & 0xfefefefefefefefe) >> 1) | \
> + (((x) & 0x0001010101010101) << 15))
Names of these macros should say U64 or UINT64 rather than WORD. And
something to suggest that they're for alien endianness. Maybe
"LSHIFT_ALIEN_UINT64.
And UINT64_C for the constants.
> +/* Galois multiplications by 2:
> + * functions differ in shifting right or left, big- or little- endianness
> + * and by defininy polynom.
> + * r == x is allowed. */
This is a bit complex, perhaps it can be clarified a bit. We have both
the issue of big or little byte order within words. And bit order used
for representating of the polynomial: usually a less significant bit
within a byte represents a coefficient for a smaller power of the
polynomial variable x, but one of the algorithms (I can't recall which
one) uses opposite bit order.
And if I remember correctly, they all use the same polynomial, but due
to bit-order differences, there are two different ways to represent it.
Which of the functions are called with more than one constant for the
poly argument?
And "defining" is misspelled.
> +#if WORDS_BIGENDIAN
> +static inline void
> +block16_lshift_be (union nettle_block16 *dst,
> + const union nettle_block16 *src,
> + uint64_t poly)
> +{
> + uint64_t carry = src->u64[0] >> 63;
> + dst->u64[0] = (src->u64[0] << 1) | (src->u64[1] >> 63);
> + dst->u64[1] = (src->u64[1] << 1) ^ (poly & -carry);
> +}
> +#else /* !WORDS_BIGENDIAN */
There will be less clutter if all code for #if WORDS_BIGENDIAN is
grouped together. And I think I prefer "mulx" rather than "shift"
somewhere in the name, to indicate that it's not a plain shift.
> --- a/cmac.c
> +++ b/cmac.c
> @@ -44,32 +44,16 @@
>
> #include "memxor.h"
> #include "nettle-internal.h"
> -#include "cmac-internal.h"
> #include "block-internal.h"
> #include "macros.h"
>
> /* shift one and XOR with 0x87. */
> -#if WORDS_BIGENDIAN
> -void
> -_cmac128_block_mulx(union nettle_block16 *dst,
> - const union nettle_block16 *src)
> -{
> - uint64_t carry = src->u64[0] >> 63;
> - dst->u64[0] = (src->u64[0] << 1) | (src->u64[1] >> 63);
> - dst->u64[1] = (src->u64[1] << 1) ^ (0x87 & -carry);
> -}
> -#else /* !WORDS_BIGENDIAN */
> -#define LE_SHIFT(x) ((((x) & 0x7f7f7f7f7f7f7f7f) << 1) | \
> - (((x) & 0x8080808080808080) >> 15))
> -void
> +static inline void
> _cmac128_block_mulx(union nettle_block16 *dst,
> const union nettle_block16 *src)
> {
> - uint64_t carry = (src->u64[0] & 0x80) >> 7;
> - dst->u64[0] = LE_SHIFT(src->u64[0]) | ((src->u64[1] & 0x80) << 49);
> - dst->u64[1] = LE_SHIFT(src->u64[1]) ^ (0x8700000000000000 & -carry);
> + block16_lshift_be(dst, src, 0x87);
> }
> -#endif /* !WORDS_BIGENDIAN */
I think it's clearer to delete this and similar wrappers.
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
