This makes subsequent calls to 'SEC_DeletePermCRL' [using the 'CERTSignedCrl' returned by 'SEC_FindCrlByName'] fail. This blocks PSM bug# 130979
Good catch, We must be getting CRLs some other way when delete *was* working for CRLs. It looks like we have a similar problem in PK11_LookupCrls. Also it looks like SEC_DeletePermCRL is not returning an erro code on failure. I'll supply patches for these problems as well. bob
Rangen, how are you freeing the CRLHeadNodes from PK11_LookupCrls? bob
By calling PORT_FreeArena(head->arena, PR_FALSE);
Comment on attachment 74131 [details] [diff] [review] Suggested Patch >+ if(crlHandle != CK_INVALID_HANDLE) >+ crl->pkcs11ID = crlHandle; Can we simply say crl->pkcs11ID = crlHandle; ? Do we not want to override the old value of crl->pkcs11ID if crlHandle is invalid?
it should assert, or completely fail. assert is probably better, since PK11_FindCrlByName does not return the DERCrl unless the crlHandle is valid. bob
OK, that's what I was afraid of. It looks like you can't call SEC_DeletePermCert on any of these CRLs. If I setup the slot and handle, then we will leak references to the slot whenever we call this function. bob
OK, I'll update the patch to assert ....
We should not use assertions to check for invalid input arguments. Invalid input arguments should be reported with a failure status and the appropriate error code. We should only use assertions to detect internal errors or when there is no way to report an error (e.g, in a function that returns void).
It's not an input value, it's generated by a function which we call withint this function to retrieve the crl. We should either assert or fail outright, though it would be an internal error if we ever hit this condition.
Created attachment 74361 [details] [diff] [review] updated patch I agree ... if PK11_FindCrlByName returns non-null, and crlhandle is still invalid, it is an internal error - and it would make sense putting an assert right there...
Comment on attachment 74361 [details] [diff] [review] updated patch Patch looks good rangen r=relyea
I checked in Rangan's patch.
Thanks .... small correction, Bob - its Rangan, not Rangen :)