Skip to content

Comments

Apply non-time-sensitive prerequisites for clang-format to 3.6#29243

Closed
bob-beck wants to merge 8 commits intoopenssl:openssl-3.6from
bob-beck:pre-clang-format-3.6-anytime
Closed

Apply non-time-sensitive prerequisites for clang-format to 3.6#29243
bob-beck wants to merge 8 commits intoopenssl:openssl-3.6from
bob-beck:pre-clang-format-3.6-anytime

Conversation

@bob-beck
Copy link
Contributor

This does the preparatory work for OPENSSL-3.6 to prepare for clang-format.

This is the 3.6 version of #29241

This is not sensitive to the repository being frozen, as it does not yet make format changes to any file. All this does is add the clang-format file, and clean up some of our things that get in the way of reformatting the source code.

For 1657

And can be done across branches to reduce the day-of-reformat load.

!--
Thank you for your pull request. Please review these requirements:

Contributors guide: https://github.com/openssl/openssl/blob/master/CONTRIBUTING.md

Other than that, provide a description above this comment if there isn't one already

If this fixes a GitHub issue, make sure to have a line saying 'Fixes #XXXX' (without quotes) in the commit message.
-->

Checklist
  • documentation is added or updated
  • tests are added or updated

In particular fix the regex magic to be tolerant of different ways
of formatting a main program.

My past life had forgotten this magic 14 years ago when we converted
it to just a table of commands in the forks.

https://www.youtube.com/watch?v=mWbbjvYmN8A
(in it's final form it will work with either compiler
because it's currently one line, but was tripped up before
by the #ifdef, so redid it to be consistent with the
other changes previously in this stack)

While I am here correct the test to test for all possible
return values of ERR_get_error_all, without the #ifdefs
Similar to the previous errtest.c fix this also is not broken
by any reformatting today, but this change makes this follow
the same pattern as the other things that test OPENSSL_LINE
after the fact so we maintain the same paradigm everywhere.
They are not asm, but spit out stuff that is not C

Clang-format gets confused and does bad things with them.
If OPENSSL_LINE ends up on a different line than the following call here,
this test breaks.

We should perhaps reconsider if testing the reporting of OPENSSL_LINE
is what we want in a malloc test, but that's for another time than now.
we assume these to be order sensitive and not self contained, so
as per our new style we disable clang format around them.

we should consider renaming to .inc, or doing away with some
of these and just putting the code inline, but that's for
later consideration.
@bob-beck bob-beck force-pushed the pre-clang-format-3.6-anytime branch from 2b1d0bc to 9e982c6 Compare November 28, 2025 19:18
@bob-beck bob-beck mentioned this pull request Nov 28, 2025
2 tasks
@bob-beck bob-beck marked this pull request as ready for review December 1, 2025 17:24
@bob-beck bob-beck added the extended tests Run extended tests in CI label Dec 2, 2025
@bob-beck bob-beck closed this Dec 2, 2025
@bob-beck bob-beck reopened this Dec 2, 2025
@t8m t8m added approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing branch: 3.6 Applies to openssl-3.6 labels Dec 3, 2025
@t8m
Copy link
Member

t8m commented Dec 3, 2025

ping @nhorman @Sashan

@t8m t8m 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 3, 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 Dec 4, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@nhorman
Copy link
Contributor

nhorman commented Dec 4, 2025

merged, thank you!

@nhorman nhorman closed this Dec 4, 2025
openssl-machine pushed a commit that referenced this pull request Dec 4, 2025
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29243)
openssl-machine pushed a commit that referenced this pull request Dec 4, 2025
In particular fix the regex magic to be tolerant of different ways
of formatting a main program.

My past life had forgotten this magic 14 years ago when we converted
it to just a table of commands in the forks.

https://www.youtube.com/watch?v=mWbbjvYmN8A

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29243)
openssl-machine pushed a commit that referenced this pull request Dec 4, 2025
(in it's final form it will work with either compiler
because it's currently one line, but was tripped up before
by the #ifdef, so redid it to be consistent with the
other changes previously in this stack)

While I am here correct the test to test for all possible
return values of ERR_get_error_all, without the #ifdefs

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29243)
openssl-machine pushed a commit that referenced this pull request Dec 4, 2025
Similar to the previous errtest.c fix this also is not broken
by any reformatting today, but this change makes this follow
the same pattern as the other things that test OPENSSL_LINE
after the fact so we maintain the same paradigm everywhere.

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29243)
openssl-machine pushed a commit that referenced this pull request Dec 4, 2025
They are not asm, but spit out stuff that is not C

Clang-format gets confused and does bad things with them.

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29243)
openssl-machine pushed a commit that referenced this pull request Dec 4, 2025
If OPENSSL_LINE ends up on a different line than the following call here,
this test breaks.

We should perhaps reconsider if testing the reporting of OPENSSL_LINE
is what we want in a malloc test, but that's for another time than now.

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29243)
openssl-machine pushed a commit that referenced this pull request Dec 4, 2025
we assume these to be order sensitive and not self contained, so
as per our new style we disable clang format around them.

we should consider renaming to .inc, or doing away with some
of these and just putting the code inline, but that's for
later consideration.

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29243)
openssl-machine pushed a commit that referenced this pull request Dec 4, 2025
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29243)
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.6 Applies to openssl-3.6 extended tests Run extended tests in CI 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.

Prepare WebKit reformat for master and all appropriate branches.

4 participants