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

Apply find_* function refactorings #149

Merged
merged 5 commits into from
Aug 7, 2024
Merged

Conversation

clumens
Copy link

@clumens clumens commented Jul 22, 2024

This is the next batch of refactorings from PR #82, though there are a couple differences from what's in those patches:

  • Note the FOREACH_NODE -> _FOREACH_NODE stuff. Basically, I renamed it and added a new version that takes an additional config pointer argument so I don't have to update all callers everywhere at once, along with all their callers, and so on.

  • These patches built, but would crash because message_recv has a new parameter, but that function is only ever indirectly called through function pointers, and the use of those pointers did not pass the config pointer. So, various other places had to get that argument added. I did not check very hard to see how this gets handled in the original PR in later patches.

  • I added a function pointer type to simplify having to deal with the above.

clumens and others added 5 commits July 22, 2024 14:58
Future commits will continue to get rid of uses of the global config
variables.  Some of those uses involve these macros, so we need a
version of them that takes a conf argument as well.

This commit just temporarily renames those macros so not every caller
has to change.  A later commit can introduce three argument versions of
these macros, and then we can move callers in several commits.  When all
are moved, these renamed versions can go away.
These take an additional argument that is the config object.  Nothing
uses these at the moment - I'm going to move callers from the old
renamed versions to these over a bunch of commits.
...as well as functions that call it.  Also, change the site argument to
be const.

Co-authored-by: Jan Pokorný <[email protected]>
...as well as functions that call it.

Co-authored-by: Jan Pokorný <[email protected]>
...as well as functions that call it.  Also, mark _find_myself as static
and remove the unnecessary function prototype for it.

Co-authored-by: Jan Pokorný <[email protected]>
@clumens clumens requested a review from jfriesse July 22, 2024 19:09
Copy link
Member

@jfriesse jfriesse left a comment

Choose a reason for hiding this comment

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

ACK

(I'm really sorry for review taking such long time - I've simply forgot ;). In the future don't hesitate to ping me if I don't review PR it in let's say one week)

@clumens
Copy link
Author

clumens commented Aug 7, 2024

ACK

(I'm really sorry for review taking such long time - I've simply forgot ;). In the future don't hesitate to ping me if I don't review PR it in let's say one week)

No problem, will do. I'd actually kind of forgotten too. Thanks!

@clumens clumens merged commit abb7ab0 into ClusterLabs:main Aug 7, 2024
1 check passed
@clumens clumens deleted the find-globals branch August 7, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants