Diego Biurrun <[email protected]> writes:

> On Sun, Jul 03, 2011 at 06:24:13PM +0100, Mans Rullgard wrote:
>> Signed-off-by: Mans Rullgard <[email protected]>
>> ---
>>  libavutil/md5.c |  109 
>> +++++++++++++++++++++++++++++++------------------------
>>  1 files changed, 61 insertions(+), 48 deletions(-)
>
> OK
>
> ..optional suggestions below..
>
>> --- a/libavutil/md5.c
>> +++ b/libavutil/md5.c
>> @@ -72,42 +72,49 @@ static const uint32_t T[64] = { // T[i]= 
>> fabs(sin(i+1)<<32)
>>  
>> +#define CORE(i, a, b, c, d) do {                                        \
>> +        t = S[i >> 4][i & 3];                                           \
>> +        a += T[i];                                                      \
>> +                                                                        \
>> +        if (i < 32) {                                                   \
>> +            if (i < 16) a += (d ^ (b & (c ^ d))) + X[       i  & 15];   \
>> +            else        a += (c ^ (d & (c ^ b))) + X[(1 + 5*i) & 15];   \
>> +        } else {                                                        \
>> +            if (i < 48) a += (b ^ c ^ d)         + X[(5 + 3*i) & 15];   \
>> +            else        a += (c ^ (b | ~d))      + X[(    7*i) & 15];   \
>> +        }                                                               \
>> +        a = b + (a << t | a >> (32 - t));                               \
>> +    } while (0)
>> +
>
> Lines could or could not be broken after the if/else.

In this instance I actually think it's easier to read on single lines.

>> -#define CORE2(i) CORE(i,a,b,c,d) CORE((i+1),d,a,b,c) CORE((i+2),c,d,a,b) 
>> CORE((i+3),b,c,d,a)
>> -#define CORE4(i) CORE2(i) CORE2((i+4)) CORE2((i+8)) CORE2((i+12))
>> -CORE4(0) CORE4(16) CORE4(32) CORE4(48)
>> +#define CORE2(i)                                                        \
>> +    CORE( i,   a,b,c,d); CORE((i+1),d,a,b,c);                           \
>> +    CORE((i+2),c,d,a,b); CORE((i+3),b,c,d,a)
>> +#define CORE4(i) CORE2(i); CORE2((i+4)); CORE2((i+8)); CORE2((i+12))
>> +    CORE4(0); CORE4(16); CORE4(32); CORE4(48);
>>  #endif
>
> spaces after ',' would help here, possibly around '+'

I don't really care, and I'm lazy.

-- 
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to