Some fairly minor comments:
Simon Josefsson <[EMAIL PROTECTED]> writes:
> +#define MOD256(n) ((n) & (ARCFOUR_SBOX_SIZE - 1))
That is a strange name to use. I expected the definiens to be ((n) &
255). Why not use the name MOD_ARCFOUR_SBOX_SIZE, or some shorter
variant perhaps?
If the code going to say MOD256 elsewhere we might as well just
open-code the &255 and dispense with the macro entirely.....
> + register size_t i = context->idx_i;
> + register size_t j = context->idx_j;
These days there's no need for 'register'. Similarly for the
other occurrences of 'register'. It just clutters up the code.
> + register char *sbox = context->sbox;
For consistency, shouldn't arcfour_setkey also have this internal
variable? (I doubt whether it'll affect the generated code, if
optimization is enabled; it's just a style thing.)
> + i = MOD256(i + 1);
> + j = MOD256(j + sbox[i]);
GNU style suggests a space before the paren, even for macro calls.
Similarly for the other occurrences of "MOD256(".
> +#define ARCFOUR_SBOX_SIZE 256
> +
> +typedef struct
> +{
> + size_t idx_i, idx_j;
> + char sbox[ARCFOUR_SBOX_SIZE];
> +} arcfour_context;
Why must these be in arcfour.h? Shouldn't all this private to
arcfour.c?
You can replace the above lines with 'struct arcfour_context;' and
then move them into arcfour.c (replacing 'arcfour_context' with
'struct arcfour_context' everywhere).
_______________________________________________
bug-gnulib mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-gnulib
