Re: patch adding a pidfile-option to script(1)

[Robert Elz <kre%munnari.OZ.AU@localhost>]

There are more issues that need fixing as well.

The proposed:

+static void
+scriptreopen(int signo)
+{
+	(void)fclose(fscript);
+	outcc = 0;
+	fscript = fopen(fname, fmode);
+	if (outcc) {
+		(void)fflush(fscript);
+		outcc = 0;
+	}
+}


needs the "outcc = 0" line moved before the fclose(), otherwise
if a SIGALRM happens to occur while in the middle of the fclose()
then the (existing) scriptflush() will attempt an fflush() while
fclose() is running (on the same FILE*) which can't be good (doing
that fflush() from a signal handler is probably not good either).
But setting outcc=0 first causes the scriptflush() (which might
still happen) to be a no-op, it does nothing if outcc == 0, and would
be harmless if it happens to be run just as, or before, that outcc=0
is happening in scriptreopen().

I'm also not sure what the

	if (outcc) { ... }

block at the end of scriptreopen() is all about - the only place outcc
is ever set to a non-zero value is in the loop in dooutput(), and the
that is one section of code we know is not running at the same instant
as the signal handler.   That is, there is no way that outcc can be
anything except 0 (set earlier in scriptreopen()), so that code does nothing.

There's another issue as well, the fscript FILE * is opened in the parent
process, and passed down through fork()s to doutput() - that's all OK,
but after all this is setup, the parent process is doing:

        if (!rawout)
                (void)fclose(fscript);
        while (!die && (scc = read(STDIN_FILENO, ibuf, BUFSIZ)) > 0) {
                cc = (size_t)scc;
                if (rawout)
                        record(fscript, ibuf, cc, 'i');
                (void)write(master, ibuf, cc);
        }

"rawout" is the -r flag - when that's not set, there would be no issue,
but when it is, that loop is still writing everything that is read from
script's stdin to fscript - the original opened file - until script is
ready to terminate.   That loop is never going to see any SIGUSR1, and
you wouldn't want two processes re-opening the file anyway.   I'm not
sure this one can be fixed easily, and just saying "don't use -r and SIGUSR1"
isn't really a good solution -- nb: just not permitting -P and -r together
isn't enough as the code is written, the SIGUSR1 support is there whether
-P is used or not.

[Note: even though it looks like it might from the call, record() doesn't
use stdio, so there's no issue with both processes writing stdio buffers
at different times, and as long as the child process doesn't alter the
file descriptor (as in calling scriptreopen()) they are sharing a file table
entry, hence, the write offset, so they also won't be overwriting outputs)].

kre