rust-lang / rust-installer

The Bourne shell installer used by Rust and Cargo
Apache License 2.0
61 stars 68 forks source link

Quote every String variable #3

Open postmodern opened 9 years ago

postmodern commented 9 years ago

To prevent unwanted shell word splitting, it is a best practice to literally quote every String variable.

Source: http://wiki.bash-hackers.org/scripting/style#parameter_expansion

brson commented 9 years ago

I just read through install-template.sh and couldn't find any instances of this problem. If you see instances of this in the code let me know.

brson commented 9 years ago

Oh, on second review I did find that PATH, LD_LIBRARY_PATH and DYLD_LIBRARY_PATH were unquoted.

postmodern commented 9 years ago

every variable and assignment should be quoted, except for maybe internal bash variables such as $? which are always integers. Example: https://github.com/brson/rust-installer/blob/296f7adc495baf6c45dadcddb84f2c76f4799feb/install-template.sh#L49

brson commented 9 years ago

What about a case like

for available_component in $COMPONENTS; do
    ...
done

If I quote $COMPONENTS won't for then only iterate over one thing?

postmodern commented 9 years ago

Depends whether $COMPONENTS is a String or an Array. If it's a String, then you probably want implicit shell word splitting. If it's an Array, use "${COMPONENTS[@]}". Quoting Array expansions prevents the shell from implicitly shellword splitting each element.