Skip to content

Conversation

ifranzki
Copy link
Contributor

Since 9a78828 it should now build warning-free on s390x, so remove the '-Wno-stringop-overflow' build option for s390x builds.

If newly added code causes -Wstringop-overflow warnings again, it should be noted in the CI runs and the newly added code should be fixed accordingly.

References: #27710 (comment)

Since openssl@9a78828
it should now build warning-free on s390x, so remove the '-Wno-stringop-overflow'
build option for s390x builds.

If newly added code causes -Wstringop-overflow warnings again, it should
be noted in the CI runs and the newly added code should be fixed accordingly.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer triaged: feature The issue/pr requests/adds a feature tests: exempted The PR is exempt from requirements for testing labels Jun 11, 2025
Copy link
Contributor

@shahsb shahsb left a comment

Choose a reason for hiding this comment

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

This is indeed a great improvement to the codebase.

Ideally, we should always aim to fix such issues directly, rather than introducing flags that suppress overflow warnings.

Thanks, @ifranzki !!

@ifranzki
Copy link
Contributor Author

Friendly ping :-)

@t8m t8m requested a review from a team June 16, 2025 13:33
@tom-cosgrove-arm tom-cosgrove-arm 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 Jun 16, 2025
@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 Jun 17, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Jun 18, 2025
Since 9a78828
it should now build warning-free on s390x, so remove the '-Wno-stringop-overflow'
build option for s390x builds.

If newly added code causes -Wstringop-overflow warnings again, it should
be noted in the CI runs and the newly added code should be fixed accordingly.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #27803)
@t8m
Copy link
Member

t8m commented Jun 18, 2025

Merged to the master branch. Thank you for your contribution.

@t8m t8m closed this Jun 18, 2025
MichaelA-Fireblocks pushed a commit to MichaelA-Fireblocks/openssl that referenced this pull request Jul 15, 2025
Since openssl@9a78828
it should now build warning-free on s390x, so remove the '-Wno-stringop-overflow'
build option for s390x builds.

If newly added code causes -Wstringop-overflow warnings again, it should
be noted in the CI runs and the newly added code should be fixed accordingly.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#27803)
MichaelA-Fireblocks pushed a commit to MichaelA-Fireblocks/openssl that referenced this pull request Jul 15, 2025
Since openssl@9a78828
it should now build warning-free on s390x, so remove the '-Wno-stringop-overflow'
build option for s390x builds.

If newly added code causes -Wstringop-overflow warnings again, it should
be noted in the CI runs and the newly added code should be fixed accordingly.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#27803)
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: master Merge to master branch tests: exempted The PR is exempt from requirements for testing triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants