Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix a misuse of NULL check #17357

Closed
wants to merge 1 commit into from
Closed

Fix a misuse of NULL check #17357

wants to merge 1 commit into from

Conversation

liwg06
Copy link
Contributor

@liwg06 liwg06 commented Dec 27, 2021

Fixes: #17356

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Dec 27, 2021
@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Dec 27, 2021
kaduk
kaduk approved these changes Dec 27, 2021
View reviewed changes
Copy link
Contributor

@kaduk kaduk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change seems to be good on the face of it, but there's always some risk that other consumers have come to depend on the weird behavior. It seems unlikely that we will detect any with our current test suite, though...

@kaduk kaduk added approval: otc review pending This pull request needs review by an OTC member branch: 3.0 Merge to openssl-3.0 branch branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug labels Dec 27, 2021
@t8m
Copy link
Member

t8m commented Dec 28, 2021

I am OK with CLA: trivial.

@t8m t8m added cla: trivial One of the commits is marked as 'CLA: trivial' approval: done This pull request has the required number of approvals and removed approval: otc review pending This pull request needs review by an OTC member labels Dec 28, 2021
@t8m
Copy link
Member

t8m commented Dec 28, 2021

@kaduk Can you please confirm if you are OK with CLA:trivial?

@kaduk
Copy link
Contributor

kaduk commented Dec 28, 2021

Yes, I am OK with CLA:trivial

@liwg06
Copy link
Contributor Author

liwg06 commented Dec 29, 2021

That's fine. Thanks @t8m @kaduk for the approval and discussion.

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@t8m t8m added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Dec 29, 2021
@t8m
Copy link
Member

t8m commented Dec 29, 2021

Merged to master and 3.0 branches. Thank you for your contribution.

@t8m t8m closed this Dec 29, 2021
openssl-machine pushed a commit that referenced this pull request Dec 29, 2021
@liwg06 @t8m
Fix a misuse of NULL check
ff7cdc1 
Fixes: #17356

CLA: trivial

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #17357)
openssl-machine pushed a commit that referenced this pull request Dec 29, 2021
@liwg06 @t8m
Fix a misuse of NULL check
db82fa2 
Fixes: #17356

CLA: trivial

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #17357)

(cherry picked from commit ff7cdc1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaduk kaduk kaduk approved these changes

@t8m t8m t8m approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch cla: trivial One of the commits is marked as 'CLA: trivial' triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

a mistake of NULL check
4 participants
@liwg06 @t8m @kaduk @openssl-machine