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

comm: add protection for NULL info #13109

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hppritcha
Copy link
Member

related to #13108

related to open-mpi#13108

Signed-off-by: Howard Pritchard <howardp@lanl.gov>
@@ -1451,7 +1451,7 @@ static int ompi_comm_idup_internal (ompi_communicator_t *comm, ompi_group_t *gro
}

// Copy info if there is one.
{
if (info != NULL) {
Copy link
Member

@edgargabriel edgargabriel Feb 21, 2025

Choose a reason for hiding this comment

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

I am not sure I understand why this check is required. ompi_info_memkind_copy_or_set already has check internally that recognizes if the info object is NULL, see https://github.com/open-mpi/ompi/blob/main/ompi/info/info_memkind.c#L560.

That being said, I see a potential issue a few lines further down in that the argument list of memkind_copy_or_set should take &comm->instance->super, instead of &comm->super. It looks like I overlooked this location when updating the behavior to retrieve the value from the instance (instead of the parent communicator)

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.

2 participants