Prevent directly including some internal subheader files #4000
+917
−727
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of including any of the pcmki/pcmki_* headers, you should include the top-level pacemaker-internal.h instead. And then fix the one place that wasn't following this rule. Ref T777
...and remove a couple that are no longer needed.
This is the first part of a project to reduce crm/common/internal.h to a file that basically just includes all the other internal files. After that, we can then use the #error technique to make sure the subheaders are not included anywhere. But first, we need to move everything out of this header to avoid circular imports. Ref T777
This provides a place for internal mainloop related functions from crm/common/internal.h to be moved to. Ref T777
This provides a place for internal flag-related functions from crm/common/internal.h to be moved to. Ref T777
This provides a place for internal memory elated functions from crm/common/internal.h to be moved to. I've also moved pcmk__mem_assert from results_internal.h to this file since it feels memory related, but I've left pcmk__assert and pcmk__abort_as. Those don't really belong in that file, but I can't think of anywhere else to put them at the moment. Ref T777
This provides a place for internal procfs-related functions from crm/common/internal.h to be moved to. Ref T777
...and into scores_internal.h where they belong. Ref T777
...and into lists_internal.h where they belong. Ref T777
This provides a place for internal miscellaneous functions from crm/common/internal.h to be moved to. I kind of hate ever having a file named utils, but this lines up with the C file in lib/common/. Ref T777
This provides a place for internal process-related functions from crm/common/internal.h to be moved to. Ref T777
...and into failcounts_internal.h where they belong. Additionally, logging.h needs to include results.h because CRM_CHECK uses crm_abort. It would be worth going through all these header files and make sure they include everything they need, but that's a project for a different day. Ref T777
This provides a place for internal cibsecrets-related functions from crm/common/internal.h to be moved to. Ref T777
It's defined in logging.c in libcrmcommon, so this is a natural place for the extern line to go. Ref T777
…aders. A couple of the subheaders need to be fixed to avoid the circular includes, but it's much better now than it was before all those previous patches to split things out of internal.h. I'm leaving PCMK__NELEM in here because I can't think of a better place for it (maybe utils_internal.h?) and it's not really doing any harm here. I've intentionally left out a couple headers: * unittest_internal.h - This is specific to unit testing and doesn't really have anything to do with public vs. private API. The _internal.h part of its name is mostly meant to make sure it's not included in the source distribution by Makefile.am * various XML headers - xml_internal.h is meant to wrap including all of those, so they don't need to be separately listed. Ref T777
Now that crm/common/internal.h includes all the private header files, crm_internal.h can include just that one. Ref T777
...headers directly. Instead of including any of the crm/common/*_internal.h headers, you should include the top-level crm/common/internal.h instead. And then fix the many, many places that were including these headers. Note that the #ifdef/#error/#endif blocks need to go above any of the multiple inclusion guards. This is because every file includes crm_internal.h first, which includes crm/common/internal.h, which includes all the internal headers, which causes the multilple inclusion guards to be set. Then when those internal headers are included directly later on, the guards will prevent the file from being included and also prevent the #error from happening. Ref T777
This needs to be defined or else testing internal headers will fail due to the #error preprocessor directives.
clumens
changed the title
Contributor
Author
|
This is an experiment to see if we can easily have a library-level main internal header file that includes all the subheaders, and then prevent anyone from directly including those subheaders. It's not foolproof, of course, but it just needs to be good enough to prevent us from making mistakes during development. I only converted two of the private library-level header files over, and only did a little bit of reorganization of #include blocks. There's an almost endless amount of cleanups that could be done on those blocks, and I don't think it's worth doing one giant PR that fixes it all. We can deal with that stuff as we go along. I think especially with |
clumens
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.