[PATCH v2] tests: Add test for decoding of PTP_* ioctl commands

[Dmitry V. Levin]

On Tue, Feb 27, 2018 at 04:38:52PM +0530, Harsha Sharma wrote:
> On Mon, Feb 26, 2018 at 7:47 PM, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > On Mon, Feb 26, 2018 at 06:20:22PM +0530, Harsha Sharma wrote:
> > [...]
> >> +#include "tests.h"
> >> +#include "xlat.h"
> >> +#include <fcntl.h>
> >> +#include <stdio.h>
> >> +#include <stdlib.h>
> >> +#include <sys/ioctl.h>
> >> +#include <inttypes.h>
> >> +#include <unistd.h>
> >> +
> >> +#ifdef HAVE_STRUCT_PTP_SYS_OFFSET
> >
> > I don't think this is going to build if the macro is not defined.
> > See many examples e.g. tests/ioctl_mtd.c how to do it properly.
> >
> > [...]
> >> +     /* PTP_PEROUT_REQUEST */
> >> +     ioctl(-1, PTP_PEROUT_REQUEST, NULL);
> >> +     printf("ioctl(-1, PTP_PEROUT_REQUEST, NULL) = -1 EBADF (%m)\n");
> >> +     ioctl(-1, PTP_PEROUT_REQUEST, perout);
> >> +     printf("ioctl(-1, PTP_PEROUT_REQUEST, {start={%" PRId64 ""
> >> +            ", %" PRIu32 "}, period={%" PRId64 ", %" PRIu32 "}"
> >> +            ", index=%d, flags=", (int64_t)perout->start.sec,
> >> +            perout->start.nsec, (int64_t)perout->period.sec,
> >> +            perout->period.nsec, perout->index);
> >
> > From this expected output I can tell that the decoder itself
> > should be fixed to print field names along with their values.
> 
> Do you mean, printing it in this way: {start={sec=%" PRId64 ", nsec=%"
> PRIu32 "}, period={sec=%" PRId64 ",  nsec=%" PRIu32 "}", index=%d,
> flags=" ?

Yes, or maybe use macros from print_fields.h

> If it is so, should I change it in same patch ?

A fix of decoder doesn't have to be in the same commit with a new test.

> > [...]
> >> +     /* PTP_CLOCK_GETCAPS */
> >> +     int getcaps_ret = ioctl(fd, PTP_CLOCK_GETCAPS, caps);
> >> +     printf("ioctl(%d, PTP_CLOCK_GETCAPS, %p) = %s\n",
> >> +            fd, caps,  sprintrc(getcaps_ret));
> >
> > What if PTP_CLOCK_GETCAPS actually succeeds?
> >
> >> +     /* PTP_SYS_OFFSET */
> >> +     int sysoff_ret = ioctl(fd, PTP_SYS_OFFSET, sysoff);
> >> +     printf("ioctl(%d, PTP_SYS_OFFSET, {n_samples=%u}) = %s\n",
> >> +            fd, sysoff->n_samples, sprintrc(sysoff_ret));
> >
> > Likewise, what if you specified sysoff->n_samples <= PTP_MAX_SAMPLES
> > and the call actually succeeded?
> 
> Should I take this case in test, beacuse other tests like ioctl_mtd
> don't consider this case ?

As you may have noticed already, strace test suite is not 100% complete,
in many tests there is a room for improvement.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20180227/8440a52e/attachment.bin>