[PATCH v1 4/4] tests: check decoding of successful quotactl

[Eugene Syromiatnikov]

On Tue, Jul 10, 2018 at 09:57:35PM +0800, Zhibin Li wrote:
> On Tue, Jul 10, 2018 at 7:49 PM Dmitry V. Levin <ldv at altlinux.org> wrote:
> 
> > On Mon, Jul 09, 2018 at 12:11:32AM +0800, Zhibin Li wrote:
> > [...]
> > > diff --git a/tests/quotactl.h b/tests/quotactl.h
> > > index 352f9270..087747cc 100644
> > > --- a/tests/quotactl.h
> > > +++ b/tests/quotactl.h
> > > @@ -78,6 +78,7 @@ enum check_quotactl_flags {
> > >       CQF_ADDR_CB   = 1 << CQF_ADDR_CB_BIT,
> > >  };
> > >
> > > +static const char *errstr;
> > >
> > >  static inline void
> > >  check_quota(uint32_t flags, int cmd, const char *cmd_str,
> > > @@ -116,6 +117,21 @@ check_quota(uint32_t flags, int cmd, const char
> > *cmd_str,
> > >       va_end(ap);
> > >
> > >       rc = syscall(__NR_quotactl, cmd, special, id, addr);
> > > +
> > > +     errstr = sprintrc(rc);
> > > +
> > > +#ifdef INJECT_RETVAL
> > > +     if (rc != INJECT_RETVAL)
> > > +             error_msg_and_fail("Got a return value of %ld != %d",
> > > +                                rc, INJECT_RETVAL);
> > > +
> > > +     static char inj_errstr[4096];
> > > +
> > > +     snprintf(inj_errstr, sizeof(inj_errstr), "%s (INJECTED)", errstr);
> > > +     errstr = inj_errstr;
> > > +     addr_cb_arg = 0;
> >
> > Why addr_cb_arg is being cleared here?
> >
> Because the condition
> if (((rc != 0) && out_arg) || (out_arg > 1)) {
>         printf("%p", qs);
>         return;
> }
> in functions print_xquota_stat() and print_xquota_statv() in
> tests/quotactl-xfs.c will be true if injection is working (rc = 42) and
> out_arg is true at the same time. addr_cb_arg is a pointer which is
> passed to paramter void *arg in the two functions mentioned above. And then
> 
> long out_arg = (long) arg;
> 
> As I see, if out_arg is not cleared in the injection, then all the
> structures
> and other fields will not be printed, which mismatches the output of strace
> itself. I thought addr_cb_arg is prepared for this purpose but am I
> misunderstaing? Is it going to be used somewhere else?

addr_cb_arg is an opaque pointer that is passed to addr_cb callback
function in case CQF_ADDR_CB is set (and no CQF_ADDR_SKIP flag
is present).  Messing with it inside check_quota is wrong.
For the injection case, the "rc != 0" checks could probably be changed
to "rc < 0" ones (with a possible additional check that rc
is equivalent to 42 if INJECT_RETVAL is present).