[PATCH v2 1/3] Option to print only failing syscalls

[Kohl, Burkhard]

> -----Original Message-----
> From: Dmitry V. Levin [mailto:ldv at altlinux.org]
> Sent: Friday, April 5, 2019 02:52
> To: Paul Chaignon <paul.chaignon at gmail.com>
> Cc: strace-devel at lists.strace.io; Kohl, Burkhard <burkhard.kohl at intel.com>
> Subject: Re: [PATCH v2 1/3] Option to print only failing syscalls
>
> On Tue, Apr 02, 2019 at 09:56:34PM +0200, Paul Chaignon wrote:
> > Existing -z option prints only successful syscalls.  This patch adds a
> > -Z option to print only failing syscalls.
> >
> > Signed-off-by: Paul Chaignon <paul.chaignon at gmail.com>
> > Co-Authored-by: Burkhard Kohl <burkhard.kohl at intel.com>
> > ---
> >  defs.h    | 1 +
> >  strace.c  | 9 +++++++--
> >  syscall.c | 6 ++++--
> >  3 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/defs.h b/defs.h
> > index 88dad64d..b5ebb343 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -417,6 +417,7 @@ extern bool iflag;  extern bool count_wallclock;
> > extern unsigned int qflag;  extern bool not_failing_only;
> > +extern bool failing_only;
> >  extern unsigned int show_fd_path;
> >  /* are we filtering traces based on paths? */  extern struct path_set
> > { diff --git a/strace.c b/strace.c index ca8d558c..bd2dd992 100644
> > --- a/strace.c
> > +++ b/strace.c
> > @@ -109,8 +109,9 @@ static bool daemonized_tracer;  static int
> > post_attach_sigstop = TCB_IGNORE_ONE_SIGSTOP;  #define use_seize
> > (post_attach_sigstop == 0)
> >
> > -/* Sometimes we want to print only succeeding syscalls. */
> > +/* Sometimes we want to print succeeding/failing syscalls only. */
> >  bool not_failing_only;
> > +bool failing_only;
> >
> >  /* Show path associated with fd arguments */  unsigned int
> > show_fd_path; @@ -304,6 +305,7 @@ Miscellaneous:\n\
> >   */
> >  /* this is broken, so don't document it  -z -- print only succeeding
> > syscalls\n\
> > +-Z -- print only failing syscalls\n\
> >   */
> >  , DEFAULT_ACOLUMN, DEFAULT_STRLEN, DEFAULT_SORTBY);
> >     exit(0);
> > @@ -1585,7 +1587,7 @@ init(int argc, char *argv[])  #ifdef
> > ENABLE_STACKTRACE
> >         "k"
> >  #endif
> > -       "a:Ab:cCdDe:E:fFhiI:o:O:p:P:qrs:S:tTu:vVwxX:yz")) != EOF) {
> > +       "a:Ab:cCdDe:E:fFhiI:o:O:p:P:qrs:S:tTu:vVwxX:yzZ")) != EOF) {
> >             switch (c) {
> >             case 'a':
> >                     acolumn = string_to_uint(optarg);
> > @@ -1716,6 +1718,9 @@ init(int argc, char *argv[])
> >             case 'z':
> >                     not_failing_only = 1;
> >                     break;
> > +           case 'Z':
> > +                   failing_only = 1;
> > +                   break;
> >             default:
> >                     error_msg_and_help(NULL);
> >                     break;
> > diff --git a/syscall.c b/syscall.c
> > index 5f95c2bd..7ea45292 100644
> > --- a/syscall.c
> > +++ b/syscall.c
> > @@ -761,8 +761,10 @@ syscall_exiting_trace(struct tcb *tcp, struct timespec *ts, int res)
> >      * whereas the intended result is that open(...) line
> >      * is not shown at all.
> >      */
> > -           if (not_failing_only && tcp->u_error)
> > -                   return 0;       /* ignore failed syscalls */
> > +           if ((not_failing_only && syserror(tcp)) ||
> > +               (failing_only && !syserror(tcp)))
> > +                   return 0;       /* ignore failed/successful
> > +                                    * syscalls */
>
> Do you think it makes sense to specify both -z and -Z options?
>
> If not, let's explicitly make them mutually exclusive.
>
>
> --
> ldv

Could it make sense as a test scenario, i.e. see what's not staged?

Regards,
Burkhard
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928