Ralf Wildenhues <[EMAIL PROTECTED]> writes:
> Hi Simon,
>
> * Simon Josefsson wrote on Fri, Oct 14, 2005 at 01:12:59AM CEST:
>> How about this?
>
> Some style comments, if I may..
Thanks!
> 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.
Added to my local copy, thanks.
>> + 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;
> }
> }
It works fine, added. Avoiding copying the key may be a good idea,
generally.
> 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. :)
Feel free to provide a patch. :) I don't have a good reference for
test vectors.
Thanks,
Simon
_______________________________________________
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
