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

Remove uses of FILENAME_MAX in the runtime #26381

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

riftEmber
Copy link
Member

@riftEmber riftEmber commented Dec 9, 2024

Replace uses of FILENAME_MAX for buffer sizes in the runtime, replacing them with heap-allocated strings of the proper size that are reallocated as needed.

Merges in and builds on the work by @rchinmay in #17059.

Completes the second part of #8757 (with #26357).

[reviewer info placeholder]

Testing:

  • paratest
  • gasnet paratest
  • C backend paratest
  • GPU tests
  • manual run of a nightly gasnet script
    • that covers affected code

rchinmay and others added 7 commits January 30, 2021 23:17
Signed-off-by: R Chinmay <[email protected]>
…me-max

New uses of FILENAME_MAX mark where HEAD switched to snprintf (with a
FILENAME_MAX buffer size), while the merged-in work used the sprintf that
was present at the time. These are to be updated shortly to use a more
sensible buffer size.

Signed-off-by: Anna Rift <[email protected]>
Also includes some formatting fixes

Signed-off-by: Anna Rift <[email protected]>
@riftEmber riftEmber force-pushed the runtime-no-filename-max branch from c6a81b4 to 57476c2 Compare December 9, 2024 19:56
@riftEmber riftEmber force-pushed the runtime-no-filename-max branch from 72b95c3 to d0fb700 Compare December 9, 2024 20:49
@riftEmber riftEmber changed the title Allow arbitrary path lengths in runtime Remove uses of FILENAME_MAX in the runtime Dec 9, 2024
@riftEmber riftEmber marked this pull request as ready for review December 9, 2024 20:50
@riftEmber riftEmber force-pushed the runtime-no-filename-max branch from b2e7363 to 47ec125 Compare December 9, 2024 20:54
@riftEmber riftEmber force-pushed the runtime-no-filename-max branch from 47ec125 to 2f0efff Compare December 9, 2024 20:55
To avoid out-of-scope change

Signed-off-by: Anna Rift <[email protected]>
@riftEmber riftEmber force-pushed the runtime-no-filename-max branch from bde7965 to 6f0911a Compare December 9, 2024 20:57
Signed-off-by: Anna Rift <[email protected]>
Signed-off-by: Anna Rift <[email protected]>
runtime/src/chpl-launcher-common.c Outdated Show resolved Hide resolved
runtime/src/chpl-launcher-common.c Outdated Show resolved Hide resolved
runtime/src/chpl-launcher-common.c Outdated Show resolved Hide resolved
runtime/src/chpl-launcher-common.c Outdated Show resolved Hide resolved
runtime/src/launch/slurm-srun/launch-slurm-srun.c Outdated Show resolved Hide resolved
@riftEmber riftEmber force-pushed the runtime-no-filename-max branch from 4c99306 to 04c552b Compare December 16, 2024 21:51
@riftEmber riftEmber force-pushed the runtime-no-filename-max branch 18 times, most recently from 20a2f46 to 2344862 Compare December 16, 2024 23:08
@riftEmber riftEmber requested a review from jabraham17 December 16, 2024 23:13
vasslitvinov added a commit that referenced this pull request Dec 21, 2024
This safeguards spaces and most other symbols in the arguments passed to
a compiled Chapel program that uses a slurm launcher, where such a
safeguard was missing. For example:

    ./myProgram --myString='Hello, world!'

used to behave as if it were `./myProgram --myString=Hello, world!`,
i.e., `world!` would be passed as a separate argument to
`myProgram_real` when launched with slurm.

This is a stopgap solution to ensure that the test augmented in #26439
passes in slurm configurations. It is not bullet-proof because user
arguments containing single quotes and/or backslashes will not be passed
through properly.

Next steps: #26381 eliminates static limits on the size of user
arguments. In the long run we want to switch from launching using
`system()`, when quoting may be necessary, to using `execvp` to avoid
the need to quote.

Discussed with Brad and Anna.
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.

4 participants