Skip to content

Conversation

300degree
Copy link
Contributor

i inspired by OpenMPI

#1185

@300degree 300degree marked this pull request as ready for review August 19, 2025 15:26
@300degree 300degree marked this pull request as draft August 19, 2025 15:28
@300degree 300degree marked this pull request as ready for review August 19, 2025 16:07
@300degree
Copy link
Contributor Author

Hi @markbrown314, apologies for the tag.
I opened this PR to fix issue. When you have some time, could you please review it? Thank you!

@markbrown314
Copy link
Collaborator

Thanks! I will take a look at as soon as I can.

@markbrown314 markbrown314 self-requested a review August 25, 2025 19:21
Copy link
Collaborator

@markbrown314 markbrown314 left a comment

Choose a reason for hiding this comment

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

Thanks for the commit. Please clean up the commit message. Here is an example you can use:

Improved SHMEM_SYMMETRIC_SIZE Validation

Issue #1185 

If SHMEM_SYMMETRIC_SIZE contains invalid characters, return error.

Signed-off by: Your Real Name <email address>

It would be best if your signed off name be your real name rather than a pseudonym. As well as the Author Name (user.name git config).

src/shmem_env.c Outdated

n = sscanf(str, "%lf%c", &p, &f);
if (2 == sscanf(str, "%lf%c%n", &p, &f, &n)) {
if (str[n] != '\0') { exit(1); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to return 1 rather than exit(1)
When the atol_scaled() returns a non-zero value it prints "Invalid size in environment variable" to user. Exit will just shut it down abruptly.

@300degree 300degree force-pushed the fix/T-1185-symmetric-size-env-validation branch from 9d4acf4 to 3a241fd Compare August 25, 2025 20:52
Issue Sandia-OpenSHMEM#1185

If SHMEM_SYMMETRIC_SIZE contains invalid characters, return error.

Signed-off-by: Kitibodee Phuphanwoe <kitibodee.ph@ksu.ac.th>
@300degree 300degree force-pushed the fix/T-1185-symmetric-size-env-validation branch from 3a241fd to 389f4f8 Compare August 27, 2025 01:28
@300degree
Copy link
Contributor Author

The previous one was mistyped. I forgot to use -s in git commit --amend.

Copy link
Collaborator

@markbrown314 markbrown314 left a comment

Choose a reason for hiding this comment

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

This looks good. 🎉 Thank you for the PR.

@markbrown314 markbrown314 merged commit 4aa91d7 into Sandia-OpenSHMEM:main Aug 27, 2025
61 of 62 checks passed
@300degree 300degree deleted the fix/T-1185-symmetric-size-env-validation branch August 28, 2025 11:13
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