Patch 0004: Fix quoting etc. #7
Conversation
- `ARGS=$@` splits arguments with white-spaces into multiple arguments, while `ARGS="$@"` keeps them intact. - Quoting of the `DEVICEUSER` variable allows for arbitrary user-names, e.g. with special characters. - Prefer `grep -F` when grep'ing for a fixed string (i.e., not a RegEx). - Minor style changes to increase readability (e.g. use soft quotes (double quotes) when needed, else use hard quotes (single quotes)). - Rectify the strange indention of the middle line in the multi-line sed statement.
|
This PR supersedes PR #6. |
| +ARGS="$@" | ||
| +DEVICEUSER="$(loginctl list-sessions | grep -F seat0 | tr -s ' ' | cut -d ' ' -f 4)" | ||
| +/bin/su -l "$DEVICEUSER" -c "/usr/bin/udisksctl $ARGS" |
There was a problem hiding this comment.
Hm, I am certain the quoting added to variable assignments makes no difference.
ARGS would need more to be safe, e.g.
/bin/su -l "$DEVICEUSER" -c "/usr/bin/udisksctl $(printf '%q ' "$@")"
or where bash is available, then more conveniently
/bin/su -l "$DEVICEUSER" -c "/usr/bin/udisksctl ${@@Q}"
There was a problem hiding this comment.
Hm, I am certain the quoting added to variable assignments makes no difference.
I am certain it does, see the bash man-page, section "Parameters→Special parameters→@". Other shells behave the same, IIRC this is POSIX-specified behaviour.
For the variable DEVICEUSER this is also the case, when a user-name contains characters with a special meaning for the shell, e.g., = # * ~ | & ; : ( ) < > space tab etc. The "quoting is superfluous" line of thinking resulted in many shell scripts not being able to handle spaces in file names etc. etc.; quoting never does harm, omitting it often does when special characters are used, resulting in "ominous and spurious failures" dependent on the data handled (here: the user name). But for the user-names nemo and deviceuser this sure does make no difference.
ARGS would need more to be safe, e.g.
Well that is a different enhancement you are proposing, which is beyond the scope of simply fixing the quoting. You may consider to pose a PR.
Still I wonder about the format string %q for printf, which looks wrong to me. From the man-page printf (1) (i.e., the executable): "one of diouxXfeEgGcs", i.e., no %q. From the man-page printf (3) (i.e., the C-function): "q : "quad". 4.4BSD and Linux libc5 only. Don't use. This is a synonym for ll." and "ll : (ell-ell). A following integer conversion corresponds to a long long int or unsigned long long int argument, or a following n conversion corresponds to a pointer to a long long int argument."!
Why not using a simple %s?
or where bash is available, then more conveniently
Generally bash is not installed on SailfishOS. For details see the section "Side note" of this comment.
ARGS=$@splits arguments with white-spaces into multiple arguments, whileARGS="$@"keeps them intact.DEVICEUSERvariable allows for arbitrary user-names, e.g. with special characters.grep -Fwhen grep'ing for a fixed string (i.e., not a RegEx).