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

opal,ompi: move container_of definition to its own header #12724

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

wenduwan
Copy link
Contributor

@wenduwan wenduwan commented Aug 1, 2024

Centralize container_of macro definition so that we don't have to define it everywhere.

@wenduwan wenduwan requested a review from jsquyres August 1, 2024 00:13
@wenduwan wenduwan requested a review from devreal August 1, 2024 00:48
@wenduwan wenduwan force-pushed the define_container_of branch from 9918983 to 1d112bd Compare August 1, 2024 00:50
@wenduwan
Copy link
Contributor Author

wenduwan commented Aug 1, 2024

@jsquyres I touched usnic for this PR so it would be great to have your eyes.

BEGIN_C_DECLS

#ifndef container_of
# define container_of(ptr, type, member) ((type *) (((char *) (ptr)) - offsetof(type, member)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we namespace this to opal_container_of so that there is no conflict if another project has the brilliant idea of providing their own copy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmmm container_of is quite common in the wild and we are actually not making a fancier option than the default one, so I'm inclined to keep the name(also we are doing ifndef so it should be safe).

Copy link
Contributor

Choose a reason for hiding this comment

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

I missed the #ifndef so I guess it's fine and keeps the impact small.

Centralize container_of macro definition so that we don't have to define
it everywhere.

Signed-off-by: Wenduo Wang <[email protected]>
@wenduwan wenduwan force-pushed the define_container_of branch from f462fe0 to 8acb59d Compare August 1, 2024 15:26
BEGIN_C_DECLS

#ifndef container_of
# define container_of(ptr, type, member) ((type *) (((char *) (ptr)) - offsetof(type, member)))
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed the #ifndef so I guess it's fine and keeps the impact small.

@bosilca bosilca merged commit 290953a into open-mpi:main Aug 1, 2024
13 of 14 checks passed
@wenduwan
Copy link
Contributor Author

wenduwan commented Aug 1, 2024

I'm running more tests. Will open backports today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants