-
Notifications
You must be signed in to change notification settings - Fork 830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix static analyzer possible leak #6894
Conversation
crl would never be null there but clean up code to make sure newcrl->crlLock gets free'd
Retest this please |
ret = DupX509_CRL(crl, newcrl); | ||
wc_UnLockRwLock(&newcrl->crlLock); | ||
if (ret != 0) { | ||
FreeCRL(crl, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to check crl != NULL
here like the original code did?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FreeCRL
should also check crl != NULL
and return BAD_ARG_ERR or something. It will crash if NULL is passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crl
will never be NULL
here as its checked right after allocation. I agree that FreeCRL
should be performing a check internally though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some questions that should be addressed.
crl would never be null there but clean up code to make sure newcrl->crlLock gets free'd