Re: record stream using time limit, reconnecting as needed

[Greg Wooledge]

On Thu, Feb 23, 2023 at 01:06:49AM -0500, Roger wrote:
> kill -- -$(ps -o pgid=$pgid_task | grep [0-9] | tr -d ' ')

There are so many sketchy things going on in that command... but let's
focus first on the obvious one:

You need to quote the '[0-9]' regex to avoid having the shell expand
it as a glob pattern.

Depending on your shopt settings, the glob pattern may disappear
entirely, may cause the script to abort, or may be expanded to a list
of single-digit filenames, if you're in a directory where any exist.

Quoting the regex avoids those issues.

The second thing I'd point out is that you want to remove the leading
"-" before the command substitution, since you're trying to pass a list
of PIDs, and not a -PGID followed by individual PIDs.

The third thing is that you don't actually need to remove spaces, because
you're using an unquoted command substitution.  The spaces will be removed
by word splitting, when the command sub's output is divided into words,
which become arguments to ps.

The fourth thing is that you appear to be using the wrong arguments
to ps.  The -o option is followed by an output format specifier, but
you're treating it like a process selector.  Maybe your ps command works
differently than mine, but here's what I'm seeing:

unicorn:~$ ps
    PID TTY          TIME CMD
   1010 pts/2    00:00:00 bash
2590830 pts/2    00:00:00 ps

unicorn:~$ ps -o pid,pgid,comm
    PID    PGID COMMAND
   1010    1010 bash
2590848 2590848 ps

unicorn:~$ ps -o pgid=1010
   1010
   1010
2590876

In my version of ps, "-o pgid" generates a listing containing only the
PGID column (with PGID as the column header), and appending "=foo" to
the column specifier changes the *title* of the column.

unicorn:~$ ps -o pgid=foobar
 foobar
   1010
2591044

So, "-o pgid=1010" prints that same column, but with "1010" as the header.
I believe this is not what you intended.  Unless your ps command is very
different from mine, this is a problem.

The fifth thing is related to the third thing.  You're using word
splitting to divide ps's output into words, and using grep to remove the
header (except that you've changed the header to a number, so grep isn't
actually removing that header any more).  All of this is unnecessary.
You know ps's output format, and you can make use of that.

The output format is a series of lines, with the first line being a
header that you want to ignore, and the remaining lines containing PIDs
that you'd like to retain.  Here's how I'd do it:

mapfile -s 1 -t pids < <(ps -whatever)
kill "${pids[@]}"

In other words: explicitly skip the first line (-s 1), and then read
each remaining line as an array element.  Finally, send SIGTERM to each
PID in the array.

Bash's kill command doesn't have a problem with leading spaces, either:

unicorn:~$ kill "   2591701"
unicorn:~$ 

This worked just fine.  So, there's no need to trim leading spaces from
the lines that ps prints, when loading them into the array.