On Wednesday 15 of July 2009 19:31:00 Rob Crittenden wrote:
> - line 818 prints an error that nickname wasn't specified but includes
> nickname in the argument list.

Good catch.

> - you should probably verify earlier that pRetKey is returned by
> NSS_GetClientAuthData(). Why display the cert info if there isn't a key?

It makes sense to me.

> - I'm not sure why you are getting the certificate nickname you are in
> this block:
>
> nickname = (*pRetCert)->nickname;
> if (NULL == nickname)
> nickname = "[unknown]";
>
> Why not just use the nickname that the user provided (or show both)?

The patch actually comes with a new feature. User does not need to specify any
nickname and NSS tries to find the best certificate automatically within the
NSS_GetClientAuthData() function. We then only print information about the
chosen certificate.

As for for the condition (NULL == nickname) I don't know NSS internals enough
to say if it can ever happen. Once NSS finds a certificate it might have
a nickname. But I am not sure here, it's sort of defensive programming.

> Otherwise it looks ok, sorry it took me so long to review.

No matter how long it takes, the code is now better. Thanks! Enclosed are both
(incremental and merged) patches.

Kamil