On Thu, Jul 20, 2006 at 03:26:55PM +0200, Andy Polyakov wrote:

> The time span between original submission and update suggests that 
> contributors were planning the update, meaning that the code was 
> considered work in progress all along. If so, why it went into stable 
> branch?

I understand that two different implementations were developed in
parallel, and in the end it was found that the one finished later
was found to provide better performance.

Notice that while the new code indeed is in the stable branch, it is
completely excluded from compilation with the default settings
(implicit -no_camellia).  Otherwise I would not have accepted the
patch for 0.9.8-stable.


>         Then this update quality... It's just wrong on several points. 
> Most notably
> 
> #ifdef L_ENDIAN
> ...
> #else /* big endian */
> 
> The expectation is
> 
> #ifdef L_ENDIAN
>  little-endian
> #elif defined(B_ENDIAN)
>  big-endian
> #else
>  endian *neutral*!
> #endif
> 
> I mean undefined L_ENDIAN does not mean big-endian, not in OpenSSL 
> context. Furthermore
> 
> #if (defined (__GNUC__) && !defined(i386))
> #define CAMELLIA_SWAP4(x) \
>   do{\
>     asm("bswap %1" : "+r" (x));\
>   }while(0)
> 
> So if you try to compile with gcc on non-x86 platform, it will insist on 
> injecting x86 instruction...

True, this does not make sense.

>                              Update appears as if they were trying to 
> improve performance, but I observe over 30% *degradation* on x86...

The stated reason for switching to the new implementation was
performance, however I don't know what specific platforms this applies
to.

A new patch taking into account the compliation problems found so far
(including the ones pointed out by Gisle Vanem) should be on the way ...


______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@openssl.org
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to