And only one nit from me:
ikeadm.c, 1150: A run-on sentence replaced with a shorter one and
a fragment. If it were me, I'd discombobulate it this way:
"Localization for ikeadm seems more straightforward when complete
phrases are translated rather than a part of a phrase, a call to
dump_foo(), and more of the phrase. It could also accommodate
non-English grammar more easily."
D.
Ferenc Rakoczi wrote:
> One general comment: you assume that little endian
> implies no alignment requirement for multiple byte loads.
> While this is true for the architectures we care about
> the most (x86 and amd64 and sparc),
> I think the code would be much cleaner if the
> #ifdef _LITTLE_ENDIAN lines were replaced by
> #if defined(__i386) || defined(__amd64)
> where that assumption is made.
>
> I also noticed a cut&past&edit error:
>
> dca_3des.c :
>
> 697 des_ctx->dr_ctx.iv[0] = htonl(param[0]);
> 698 des_ctx->dr_ctx.iv[1] = htonl(param[0]);
>
> In the second line it should be param[1].
>
> Otherwise, looks good, and it is great that someone did this.
>
> Ferenc
>
> Dan Anderson wrote:
>> Here's a review for these two CRs to add 64-bite ntohll() and htonll()
>> functions to Solaris
>> http://dan.drydog.com/reviews/5007142-bswap/
>>
>> I tested the performance gain of these functions implemented in inline
>> assembly and C, and the inline showed a 5x-7x gain over, depending on
>> whether it's 32- or 64-bit and on AMD64 or EM64T. Unfortunately, the
>> functions weren't used enough to show noticable performance gains in 3des,
>> although the code is simplified and byte swapping is not re-implemented
>> several times.
>>
>> Please comment by COB Wed. Aug. 27.
>>
>> Ref:
>> 5007142 Add ntohll and htonll to sys/byteorder.h
>> 6717509 Need to use bswap/bswapq for byte swap of 64-bit integer on x32/x64
>> PSARC/2008/474
>>
>> --
>> This message posted from opensolaris.org
>> _______________________________________________
>> crypto-discuss mailing list
>> crypto-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/crypto-discuss
> _______________________________________________
> crypto-discuss mailing list
> crypto-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/crypto-discuss
