-
-
Notifications
You must be signed in to change notification settings - Fork 223
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
cc_wrapper: Spill arguments back to a response file #430
base: master
Are you sure you want to change the base?
cc_wrapper: Spill arguments back to a response file #430
Conversation
… into a response file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for picking this up!
if [[ ${!i} == @* && -r "${i:1}" ]]; then | ||
# Create a temporary file that we'll spill sanitized options to since they | ||
# were originally read from a response file. | ||
temp_file=$(mktemp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do this only if the arguments have a length that gets somewhat close to the limit? Otherwise we are just doing needless file I/O.
done <"${!i:1}" | ||
mv "${temp_file}" "${!i:1}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This replaces an input file, which is something actions should avoid. Could we rewrite the argument to point to the temporary file instead?
done <"${!i:1}" | ||
mv "${temp_file}" "${!i:1}" | ||
else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The temporary file should be deleted after the compiler returns.
Fixes #421
This PR tweaks the
*cc_wraper.sh
scripts to spill command line arguments from response files, back into a file after sanitizing. Previously they were expanded into thecmd
variable which would then get specified on the command line, possibly leading to an overflow.