On Wed, 20 May 2020, Jacob Bachmeyer wrote:
> > Index: dejagnu/lib/remote.exp
> > ===================================================================
> > --- dejagnu.orig/lib/remote.exp
> > +++ dejagnu/lib/remote.exp
> > @@ -113,7 +113,10 @@ proc close_wait_program { program_id pid
> > # We reaped the process, so cancel the pending force-kills, as
> > # otherwise if the PID is reused for some other unrelated
> > # process, we'd kill the wrong process.
> > - exec sh -c "exec > /dev/null 2>&1 && kill -9 $exec_pid"
> > + #
> > + # Use `catch' in case the force-kills have completed, so as not
> > + # to cause TCL to choke if `kill' returns a failure.
> > + catch "exec sh -c \"exec > /dev/null 2>&1 && kill -9 $exec_pid\""
> > }
> >
> > return $res
> >
>
> That line is starting to look needlessly complicated. Does {catch {exec
> sh -c "kill -9 $exec_pid" >& /dev/null} also work? (I am using the
> outermost braces to denote quoting a piece of Tcl code inline in running
> text.)
I've tried that, but there is a syntax error in your proposal as the
braces are not balanced. From your note in the parentheses I infer you
meant:
{catch {exec sh -c "kill -9 $exec_pid" >& /dev/null}}
but that results in:
ERROR: (DejaGnu) proc "{catch {exec sh -c "kill -9 $exec_pid" >& /dev/null}}"
does not exist.
The error code is TCL LOOKUP COMMAND {catch {exec sh -c "kill -9 $exec_pid" >&
/dev/null}}
The info on the error is:
invalid command name "catch {exec sh -c "kill -9 $exec_pid" >& /dev/null}"
while executing
"::tcl_unknown {catch {exec sh -c "kill -9 $exec_pid" >& /dev/null}}"
("uplevel" body line 1)
invoked from within
"uplevel 1 ::tcl_unknown $args"
(which does not quite surprise me as the line is then interpreted as a
single word). This:
catch {exec sh -c "kill -9 $exec_pid" >& /dev/null}
does appear to work, including the redirection.
> The "catch" command recursively invokes the interpreter in the
> same context, so there is no need for variables to be substituted before
> invoking catch(n). As a matter of modern Tcl (8.0+) style, the script
> argument to catch(n) should be in braces. If I understand correctly, a
> braced script argument to catch(n) will be byte-compiled when the
> surrounding procedure is loaded, while other scripts must be parsed at
> runtime. When fixing bugs that only bite under heavy load, efficiency
> should be a consideration. (I have spent many hours reading the Tcl
> reference manuals; hopefully others can also benefit from my experience
> here.)
This is a rare corner case though and I would therefore recommend my
minimal fix to be applied first, and then you can apply your code
modernisation to all `exec' invocations throughout `close_wait_program'.
This way changes will be functionally consistent and won't mix fixes with
clean-ups (or indeed address independent issues at a time).
FWIW,
Maciej
