Fix bare script commands #426
Conversation
Due to the pattern matching of the '-f' option to 'pkill' in the man page example, use the full path which should avoid matching the command line of other processes. For references see: https://lists.debian.org/debian-user/2023/08/msg00406.html https://lists.debian.org/debian-user/2023/09/msg00211.html https://lists.debian.org/debian-user/2024/02/msg00800.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1053923 In short the '-f rec' option matched the '--directory' option of 'gnome-keyring-daemon' and killed it at the top of each hour when 'pkill -f rec' was run. Modifying the script and man page to use the full path (whatever that might be, typically '/usr/bin/rec' in Linux but could be elsewhere) should go along way to helping someone avoid similar pain.
Similar to the patch to 'soundlog', use the full path to 'play'.
As backticks are generally hard to see, use the synonymous $() construct to generate the name for the audio file. $() is preferred for capturing the output of a command into a shell variable.
|
Oops. Good catch.
|
|
* On 2024 21 Feb 09:38 -0600, Thomas Beierlein wrote:
Oops. Good catch.
Just two questions:
- Why also use full path to start the programs?
To make the pattern as unique as possible. Besides, it is good practice
to do so. Even if there is a locally compiled package that is early in
the user's PATH, `which` should find it.
OTOH, as `which` searches the user's PATH just like the bare `rec` as
originally written, there really is no difference.
- Did you experiment with dropping the -f parameter to pkill (as that
seems the main culprit) or substitute it by -x (exact match)?
I did experiment with dropping `-f`, but I guess I got focused on the way
I submitted the patch. Without `-f` the pattern is limited to 14
characters as I recall which is quite sufficient for `rec`.
Thanks for pointing out `-x` as I missed that one. A quick check with
`pgrep` shows that it does not match the substring in 'directory'. I
suppose that changing `-f` to `-x` in the man page template is all that
is required to spare others the same pain. Also, `-fx` appears to
prevent the substring match as well.
73, Nate
…--
"The optimist proclaims that we live in the best of all
possible worlds. The pessimist fears this is true."
Web: https://www.n0nb.us
Projects: https://github.com/N0NB
GPG fingerprint: 82D6 4F6B 0E67 CD41 F689 BBA6 FB2C 5130 D55A 8819
|
Hmm, I know these practice with complete path to NOT allow bash to pick up the first matching binary with that name in PATH - especially for security reasons..
Yes and the shell would do also. But back to the main point - the -f parameter:
I did some search in gits history. The section in the man page dates back to Rein's work before 2010. Seems that nobody (me included) did a thorough check of that part. Further in all other cases we use 'pkill()' in the source code we have no -f around. So I agree that changing -f to -x in the man page is the way to go. |
|
* On 2024 21 Feb 15:01 -0600, Thomas Beierlein wrote:
So I agree that changing -f to -x in the man page is the way to go.
Go ahead and do that, Tom.
73, Nate
…--
"The optimist proclaims that we live in the best of all
possible worlds. The pessimist fears this is true."
Web: https://www.n0nb.us
Projects: https://github.com/N0NB
GPG fingerprint: 82D6 4F6B 0E67 CD41 F689 BBA6 FB2C 5130 D55A 8819
|
|
See PR #427 |
|
Replaced by PR #427, so closing these one. Thanks Nate. |
This patch set cleans up an ambiguous example in the man page that resulted in my actual
cronentry killing thegnome-keyring-daemonon my system! By using thewhichcommand in the scripts and man page example, I hope others can avoid my annoying experience.