Review for 5031131 (performance improvement to libcryptoutil) (due July

[Dan Anderson]

I updated webrev at--please re-review:

http://dan.drydog.com/reviews/5031131-libcryptoutil/

> John Zolnowsky wrote:
> Minor comment:
> I doubt that the half-dozen instructions saved by testing
> 	if (pointer_to_free == NULL)
> before calling free() is significant.  I'd recommended calling
> 	free(pointer_to_free);
> unconditionally.
> -JZ

Removed (from action below)

>> Darren J Moffat wrote:
>>>        Alternatively have pkcs11_mech2str not do the vendor defined mech
>>>        support and have it return an error.  Which BTW is actually what
>>>        the function is defined to do and what the comment says even
>>>        though that isn't what the existing implementation did.

> Dan Anderson wrote:
>> True, it's not as clean, but it's an extra 30% performance penalty to add strdup() back in.  Always a trade-off between performance and maintenance.

Krishna Yenduri wrote:
> I like the  option of limiting pkcs11_mech2str() to the mechs in the
> standard.
> All the callers except pkcs11_kernel ones, do something like
> (ignoring error handling)
>    printf("%s", pkcs11_mech2str(mech_id));
> They can be changed to
>    if (mech_id >= CKM_VENDOR_DEFINED)
>       printf("%#lx", mech_id);
>    else
>       printf("%s", pkcs11_mech2str(mech_id));
> For pkcs11_kernel callers, which is where the performance improvement helps,
> we can keep a string buffer on the local stack and use it
> for the vendor defined case.
> So, we get to keep both the performance and a cleaner interface.
> -Krishna

I revised the code as Darren and Krishna suggested.

--
This message posted from opensolaris.org