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

hpcbind: Use double quotes around $@ when invoking user command #4284

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

mpokorny
Copy link
Contributor

@mpokorny mpokorny commented Sep 2, 2021

At the end of the hpcbind script, when running the user command, the
command arguments are in the $@ variable. Unless $@ is enclosed by
double quotes, those arguments are subject to word splitting in bash,
which creates problems when those arguments have spaces that should be
preserved. An example that would fail without this commit is hpcbind -- foo args="hello world".

At the end of the hpcbind script, when running the user command, the
command arguments are in the $@ variable. Unless $@ is enclosed by
double quotes, those arguments are subject to word splitting in bash,
which creates problems when those arguments have spaces that should be
preserved. An example that would fail without this commit is `hpcbind
-- foo args="hello world"`.
@dalg24-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

Copy link
Member

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

I think this is fine.

@dalg24 dalg24 merged commit bb26cb8 into kokkos:develop Sep 2, 2021
@mpokorny mpokorny deleted the quote-hpcbind branch September 2, 2021 22:02
@opensdh
Copy link

opensdh commented Dec 10, 2021

This fix is incomplete in that eval "$@" undoes the virtue of "$@": it concatenates the properly separated arguments with spaces and reparses the whole thing as shell input. I don't know why one would want eval here anyway, so there might be some sort of backward-compatibility concern.

@mpokorny mpokorny restored the quote-hpcbind branch December 21, 2021 21:01
mpokorny added a commit to mpokorny/kokkos that referenced this pull request Dec 21, 2021
As mentioned by @opensdh in kokkos#4284 (comment), the use of eval "$@" (in case
hwloc-bind is not used) is counter-productive. Modulo any backward compatibility
concern (which I don't see, myself), this PR addresses those concerns raised by
@opensdh.
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.

None yet

5 participants