bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: arcfour


From: Simon Josefsson
Subject: Re: arcfour
Date: Fri, 14 Oct 2005 09:57:14 +0200
User-agent: Gnus/5.110004 (No Gnus v0.4) Emacs/22.0.50 (gnu/linux)

Ralf Wildenhues <address@hidden> 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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]