Hi Simon, * Simon Josefsson wrote on Fri, Oct 14, 2005 at 01:12:59AM CEST: > How about this?
Some style comments, if I may.. > Index: lib/arcfour.c > =================================================================== > RCS file: lib/arcfour.c > diff -N lib/arcfour.c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ lib/arcfour.c 13 Oct 2005 23:12:01 -0000 *snip* > + > +#ifdef HAVE_CONFIG_H > +# include <config.h> > +#endif > + > +#include "arcfour.h" > + > +void > +arcfour_stream (arcfour_context * context, const char *inbuf, char *outbuf, > + size_t length) > +{ > + register size_t i = context->idx_i; > + register size_t j = context->idx_j; > + register unsigned char *sbox = context->sbox; > + register unsigned char t; > + > + while (length--) > + { > + i++; > + i = i & 255; /* The and-op seems to be faster than the mod-op. */ Yes (for less-than-commonly-optimizing compilers), but you could also be consistent in notation here, and two lines down the road. Furthermore, if I may suggest not to sprinkle hard-coded numbers all over the place. How about something like this #define ARCFOUR_BLOCKBITS 8 #define ARCFOUR_BLOCKSIZE (1 << ARCFOUR_BLOCKBITS) #define ARCFOUR_BLOCKMASK (ARCFOUR_BLOCKSIZE - 1) in the header, and then use the latter two consistently? If you don't like long names, maybe s/BLOCK//g. > + j += sbox[i]; > + j &= 255; > + t = sbox[i]; > + sbox[i] = sbox[j]; > + sbox[j] = t; > + *outbuf++ = *inbuf++ ^ sbox[(sbox[i] + sbox[j]) & 255]; > + } > + > + context->idx_i = i; > + context->idx_j = j; > +} > + > +void > +arcfour_setkey (arcfour_context * context, const char *key, size_t keylen) > +{ > + int i, j; > + unsigned char karr[256]; > + > + context->idx_i = context->idx_j = 0; > + for (i = 0; i < 256; i++) > + context->sbox[i] = i; > + for (i = 0; i < 256; i++) > + karr[i] = key[i % keylen]; If this were time-critical, you could hand-maintain another index k and omit the modulo. I also wonder whether merging the karr loop into the next one (and thus eliding karr completely) isn't beneficial. Like this (untested): for (i = j = k = 0; i < ARCFOUR_BLOCKSIZE; i++) { int t; j = (j + context->sbox[i] + key[k]) & ARCFOUR_BLOCKMASK; t = context->sbox[i]; context->sbox[i] = context->sbox[j]; context->sbox[j] = t; if (++k == keylen) k = 0; } } Furthermore, I guess it would be beneficial if you would test this and the other modules you recently added with test strings that are longer than respective block sizes. :) > + for (i = j = 0; i < 256; i++) > + { > + int t; > + j = (j + context->sbox[i] + karr[i]) % 256; > + t = context->sbox[i]; > + context->sbox[i] = context->sbox[j]; > + context->sbox[j] = t; > + } > +} *snip* Cheers, Ralf _______________________________________________ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib