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.

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 type error in PEM processing #5211

Closed
wants to merge 1 commit into from
Closed

Fix type error in PEM processing #5211

wants to merge 1 commit into from

Conversation

kaduk
Copy link
Contributor

@kaduk kaduk commented Jan 30, 2018

An alternative to #5200 . I can't reproduce the travis errors locally, even with clang-3.9, so making a pull request to see if it works.

The get_name() helper was using a variable of type size_t to hold the
result of BIO_gets(), but BIO_gets() returns int and makes use of negative
values to indicate error conditions.

Change the type of the local variable to match, and propagate that
through to other places in the file to avoid -Wsign-compare issues.

The get_name() helper was using a variable of type size_t to hold the
result of BIO_gets(), but BIO_gets() returns int and makes use of negative
values to indicate error conditions.

Change the type of the local variable to match, and propagate that
through to other places in the file to avoid -Wsign-compare issues.
@kaduk
Copy link
Contributor Author

kaduk commented Jan 30, 2018

I should probably mention scw00 in the commit message, sorry about the omission from the initial draft.

@kaduk
Copy link
Contributor Author

kaduk commented Feb 28, 2018

ping @openssl/omc now that the automated builds passed (which was not the case for the original proposal in #5200)

@kaduk kaduk added the approval: review pending This pull request needs review by a committer label Feb 28, 2018
@mspncp mspncp mentioned this pull request Mar 8, 2018
2 tasks
Copy link
Contributor

@richsalz richsalz left a comment

Choose a reason for hiding this comment

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

Small obvious simple change, I approve (even though we share the same employer)

levitte pushed a commit that referenced this pull request Mar 9, 2018
The get_name() helper was using a variable of type size_t to hold the
result of BIO_gets(), but BIO_gets() returns int and makes use of negative
values to indicate error conditions.

Change the type of the local variable to match, and propagate that
through to other places in the file to avoid -Wsign-compare issues.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #5211)
@kaduk
Copy link
Contributor Author

kaduk commented Mar 9, 2018

Pushed to master, closing. Thanks for the review!

@kaduk kaduk closed this Mar 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: review pending This pull request needs review by a committer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants