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

shellcheck issues suppressed that shouldn't have been #55

Open
coderobe opened this issue Dec 9, 2018 · 0 comments
Open

shellcheck issues suppressed that shouldn't have been #55

coderobe opened this issue Dec 9, 2018 · 0 comments
Assignees
Labels

Comments

@coderobe
Copy link
Member

coderobe commented Dec 9, 2018

# shellcheck disable=SC2059

# shellcheck disable=SC2059

# shellcheck disable=SC2059

# shellcheck disable=SC2059

These are valid semantic errors that can lead to printf doing the wrong thing when the variables used contain format strings. As it stands though, this behavior is expected for one of the variables used here.

I'd probably nest the printfs - one for the mesg formatting, and one for the rest.
Just in case some future terminal with more in-band signaling ends up using % in their escape sequences ;)

Alternatively, consider something like "%s ... ${mesg}" "${BOLD}" ... and whatnot, where everything but mesg is separated into printf formatting


archlinux-repro/repro.in

Lines 115 to 118 in a1dc7bd

# shellcheck disable=SC2140
mount -t overlay overlay \
-o lowerdir="$BUILDDIRECTORY/root",upperdir="$BUILDDIRECTORY/${build}_upperdir",workdir="$BUILDDIRECTORY/${build}_workdir" \
"$BUILDDIRECTORY/${build}"

This is also a valid issue. As it currently stands it looks like you may have intended for the quotes to end up in the args as literals, but right now they're all interpreted by your shell. The entire arg should be wrapped in quotes.


archlinux-repro/repro.in

Lines 259 to 260 in a1dc7bd

# shellcheck disable=SC2086
exec_nspawn build --bind="$(readlink -e ${cachedir}):/cache" pacman -U ${packages[*]} --noconfirm

Should be "${packages[@]}" instead of ${packages[*]}. What it's currently doing is basically "join all the arguments by the first character of IFS (space), split them by IFS and expand each of them as globs, and use those as args for the commandline you're building"


archlinux-repro/repro.in

Lines 319 to 328 in a1dc7bd

xdg_repro_dir="${xdg_config_home:-$home/.config}/repro"
if [[ "$repro_conf" = "$configdir/repro.conf" ]]; then
if [[ -r "$xdg_repro_dir/repro.conf" ]]; then
# shellcheck source=/dev/null
source "$xdg_repro_dir/repro.conf"
elif [[ -r "$home/.repro.conf" ]]; then
# shellcheck source=/dev/null
source "$home/.repro.conf"
fi
fi

$home is not a thing, you probably wanted $HOME

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants