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 --strict-warnings build on djgpp (3.1) #19843

Closed
wants to merge 5 commits into from

Conversation

jwt27
Copy link
Contributor

@jwt27 jwt27 commented Dec 5, 2022

Similar to #19322, but for the 3.1 branch.

For some reason djgpp uses '(unsigned) long int' for (u)int32_t.  This
causes errors with -Werror=format, even though these types are in
practice identical.

Obvious solution: cast to the types indicated by the format string.
This causes a warning otherwise when socklen_t is signed (Watt32).
This matches the declaration in <openssl/crypto.h>.
I chose to just hide this behind '#ifndef __DJGPP__', instead of listing
all the macro combinations where it *is* used.  That would make quite a
mess.
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Dec 5, 2022
@t8m t8m added approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member triaged: bug The issue/pr is/fixes a bug branch: 3.1 Merge to openssl-3.1 tests: exempted The PR is exempt from requirements for testing labels Dec 6, 2022
@t8m t8m removed the approval: otc review pending This pull request needs review by an OTC member label Dec 6, 2022
@paulidale paulidale added 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 Dec 6, 2022
@openssl-machine openssl-machine 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 8, 2022
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@paulidale
Copy link
Contributor

Merged to 3.1, thanks for the submission.

@paulidale paulidale closed this Dec 8, 2022
openssl-machine pushed a commit that referenced this pull request Dec 8, 2022
For some reason djgpp uses '(unsigned) long int' for (u)int32_t.  This
causes errors with -Werror=format, even though these types are in
practice identical.

Obvious solution: cast to the types indicated by the format string.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #19843)
openssl-machine pushed a commit that referenced this pull request Dec 8, 2022
This causes a warning otherwise when socklen_t is signed (Watt32).

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #19843)
openssl-machine pushed a commit that referenced this pull request Dec 8, 2022
This matches the declaration in <openssl/crypto.h>.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #19843)
openssl-machine pushed a commit that referenced this pull request Dec 8, 2022
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #19843)
openssl-machine pushed a commit that referenced this pull request Dec 8, 2022
I chose to just hide this behind '#ifndef __DJGPP__', instead of listing
all the macro combinations where it *is* used.  That would make quite a
mess.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #19843)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: 3.1 Merge to openssl-3.1 severity: fips change The pull request changes FIPS provider sources tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants