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

adds missing casts (#21123) #21127

Closed
wants to merge 1 commit into from
Closed

adds missing casts (#21123) #21127

wants to merge 1 commit into from

Conversation

baentsch
Copy link
Contributor

@baentsch baentsch commented Jun 5, 2023

Fixes #21123

@mattcaswell mattcaswell added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 tests: exempted The PR is exempt from requirements for testing labels Jun 5, 2023
@mattcaswell mattcaswell removed the approval: otc review pending This pull request needs review by an OTC member label Jun 5, 2023
@t8m t8m added triaged: bug The issue/pr is/fixes a bug approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Jun 5, 2023
@t8m
Copy link
Member

t8m commented Jun 5, 2023

@baentsch Please try to make the commit message somewhat more descriptive next time. No need to amend it now though, we can do it when merging.

Something like this would be better:

Cast the argument to unsigned char when calling isdigit()

Fixes #21123

@baentsch
Copy link
Contributor Author

baentsch commented Jun 5, 2023

ACK @t8m. Already was proud to remember adding reference to the issue... :-/

@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.

openssl-machine pushed a commit that referenced this pull request Jun 6, 2023
@baentsch @t8m
Cast the argument to unsigned char when calling isdigit()
8229874 
Fixes #21123

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #21127)
@t8m
Copy link
Member

t8m commented Jun 6, 2023

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

@t8m t8m closed this Jun 6, 2023
openssl-machine pushed a commit that referenced this pull request Jun 6, 2023
@baentsch @t8m
Cast the argument to unsigned char when calling isdigit()
3fbb364 
Fixes #21123

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #21127)

(cherry picked from commit 8229874)
openssl-machine pushed a commit that referenced this pull request Jun 6, 2023
@baentsch @t8m
Cast the argument to unsigned char when calling isdigit()
0ffe629 
Fixes #21123

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #21127)

(cherry picked from commit 8229874)
@tmshort tmshort mentioned this pull request Jun 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t8m t8m t8m approved these changes

@mattcaswell mattcaswell mattcaswell approved these changes

@paulidale paulidale paulidale approved these changes

Assignees
No one assigned
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 tests: exempted The PR is exempt from requirements for testing 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.

Invocations of isdigit() questionable
5 participants
@baentsch @t8m @openssl-machine @mattcaswell @paulidale