Re: arcfour

[Simon Josefsson]

Bruno Haible <address@hidden> writes:

> Simon Josefsson wrote:
>
>> +/* Initialize CONTEXT using encryption KEY of KEYLEN bytes.  KEY
>> +   should be 40 bits (5 bytes) or longer.  */
>> +extern void
>> +arcfour_setkey (arcfour_context * context, const char *key, size_t keylen);
>
> The comment should say that KEYLEN must be > 0. (If keylen==0, your
> first code would do divisions by 0, and Ralf's replacement code would do
> out-of-bounds array accesses.)

It now says:

/* Initialize CONTEXT using encryption KEY of KEYLEN bytes.  KEY
   should be 40 bits (5 bytes) or longer.  The KEY cannot be zero
   length.  */

>> +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;
>
> Usually I try to minimize the scope of variables, i.e. I would declare
> 'unsigned char t;' at the beginning of the loop. Like you are doing in
> arcfour_setkey.

Yup, I moved it.  I retained the 'register' keyword, not sure how
useful it is, but it doesn't hurt.

>> +  while (length--)
>
> If you write this as
>
>      for (; length > 0; length--)
>
> the CPU has one instruction less to execute per function call.

Incorporated too.

Ok to install?

Thanks!