[PATCH v5 3/4] Implement -e status=set option

[Paul Chaignon]

On Tue, Jul 02, 2019 at 02:26:44AM +0300, Dmitry V. Levin wrote:
> On Fri, Jun 28, 2019 at 11:34:21AM +0200, Paul Chaignon wrote:
> > The status qualifier enables filtering based on the return status of
> > syscalls.  -z and -Z become aliases for -e status=successful and -e
> > status=failed.  Staged output is only enabled when at least one status is
> > filtered, that is, when the set is incomplete.
> >
> > * defs.h (not_failing_only): Remove variable.
> > (failing_only): Remove variable.
> > (popcount32): Move function from signal.c.
> > * signal.c (popcount32): Move function to defs.h.
>
> The preferred way of writing this is:
>
> * signal.c (popcount32): Move ...
> * defs.h: ... here.
> (not_failing_only, failing_only): Remove.

Thanks.  These help.  I'll fix in other commits as well.

>
> > * filter_qualify.c (status_set): New number_set variable.
> > (statuses): New variable for names of statuses.
> > (statusstr_to_uint): New function.
> > (qualify_status): New function.
>
> (statusstr_to_uint, qualify_status): New functions.
>
> > (qual_options): Handle status qualifier.
> > * number_set.c (get_number_setbit): New function.
> > (is_complete_set): New function.
> > * number_set.h (is_complete_set): New prototype.
> > (status_t): New enumeration for statuses.
> > (status_set): Expose status_set variable.
> > * strace.1.in: Document new status qualifier.
> > * strace.c (not_failing_only): Remove variable.
> > (failing_only): Remove variable.
> > (droptcb): Handle status=detached option.
> > (init): Handle new status qualifier, set status_set variable on -z and -Z
> > options, warning on -zZ, use is_complete_set.
> > (maybe_switch_tcbs): Reopen memstream after tcb switch.
> > (print_event_exit): Handle status=unfinished option.
> > * syscall.c (syscall_entering_trace): Use is_complete_set.
> > (syscall_exiting_trace): Use is_complete_set, handle status=unavailable
> > option.
>
> The patch looks almost ready, please see my comments below.
>

[...]

> >  .TP
> > +\fB\-e\ status\fR=\,\fIset\fR
> > +Trace only system calls with the specified return status.  The default is
> > +.BR status = all .
> > +When using the
> > +.B status
> > +qualifier, because
> > +.B strace
> > +waits for system calls to return before deciding if they should be printed, the
> > +order of events may not be preserved anymore.  For example, if a first system
> > +call is being executed and another is called by a different thread,
> > +.B strace
> > +will print the first system call after the second.  The first system call will
>
> Do you mean that both entering and exiting of another syscall happen after
> entering but before exiting of the first syscall?

If that second syscall exits first, yes.  What about the following?

  For example, if two system calls are executed by concurrent threads,
  strace will first print both the entry and exit of the first system call
  to exit, regardless of their respective entry time.  The entry and exit
  of the second system call to exit will be printed afterwards.

>
> Please reword this example to make this reordering case clear.
>

[...]

> > +.TP
> > +.BR "\-e\ status" = unfinished
> > +Trace system calls that did not return.  This might happen, for example, due to
> > +an execve call in a neighbour thread.
>
> The most common case for unfinished syscalls is exit/exit_group.
> I think it worth mentioning.

I've changed the sentence to:

  This will happen, for example, with the exit/exit_group system calls or
  due to an execve call in a neighbour thread.

[...]

> >
> > +   if (zflags > 1)
> > +           error_msg("Only the second flag of -z/-Z will take effect. "
> > +                     "See status qualifier for more complex filters.");
>
> I's say that only the last of -z/-Z options will take effect.

Ah, much better.  Thanks!

[...]

Paul