On Thu, 22 Feb 2018 16:44:20 -0500, Eli Schwartz wrote:
I went a little above-and-beyond for escaping strings for the error messages in db-functions' arch_repo_add and arch_repo_remove. The code should explain itself, but I wanted to point it out, as it's more complex than the "slap %s in there, and move the ${...} to the right" that is used everywhere else.
[...] index 8b71cae..6d02c50 100644 --- a/db-functions +++ b/db-functions @@ -446,11 +446,13 @@ arch_repo_add() { local repo=$1 local arch=$2 local pkgs=(${@:3}) + local pkgs_str
# package files might be relative to repo dir pushd "${FTP_BASE}/${repo}/os/${arch}" >/dev/null + printf -v pkgs_str -- '%q ' "${pkgs[@]}" /usr/bin/repo-add -q "${repo}${DBEXT}" ${pkgs[@]} \ - || error "repo-add ${repo}${DBEXT} ${pkgs[@]}" + || error 'repo-add %q %s' "${repo}${DBEXT}" "${pkgs_str% }" popd >/dev/null set_repo_permission "${repo}" "${arch}"
@@ -462,13 +464,15 @@ arch_repo_remove() { local arch=$2 local pkgs=(${@:3}) local dbfile="${FTP_BASE}/${repo}/os/${arch}/${repo}${DBEXT}" + local pkgs_str
if [[ ! -f ${dbfile} ]]; then error "No database found at '%s'" "$dbfile" return 1 fi + printf -v pkgs_str -- '%q ' "${pkgs[@]}" /usr/bin/repo-remove -q "${dbfile}" ${pkgs[@]} \ - || error "repo-remove ${dbfile} ${pkgs[@]}" + || error 'repo-remove %q %s' "$dbfile" "${pkgs_str% }" set_repo_permission "${repo}" "${arch}"
I do see what you're doing, I'm just not sure why. Is the whole idea with this extra variable floating around, to avoid tokenizing "${pkgs[@]}" as separate messages? That's why "${pkgs[*]}" tokenizes the members of an array as one word by gluing the members together using the first IFS character (which is a space). You'll note I used this in testing2x.
As for using %q for filepaths that can theoretically contain spaces... good point I guess.
It's all about %q. (I did use ${ary[*]} somewhere else in the commit). The separate variable applies %q escaping to each package filename separately. Without it, if I did something like: + || error 'repo-remove %q %q' "$dbfile" "${pkgs[*]}" Then it would also escape the spaces that separate them. Anyway, correctly applying %q escaping probably isn't super-important. But, we don't really expect repo-add or repo-remove to fail; if something is wrong, any of the numerous checks leading up to actually calling them should have already caught that. If we trigger one of these error messages, something *weird* is going on, and I'd like the most precise error message possible. -- Happy hacking, ~ Luke Shumaker