[PATCH 0/4] Add GPIO chardev ioctl decoding

[Dmitry V. Levin]

On Wed, Dec 30, 2020 at 08:30:20AM +0800, Kent Gibson wrote:
> On Tue, Dec 29, 2020 at 11:44:42PM +0300, Dmitry V. Levin wrote:
> > On Wed, Dec 23, 2020 at 06:07:44AM +0800, Kent Gibson wrote:
> > > This patch set implements decoding of the GPIO character device ioctls
> > > first added in Linux v4.8, extended in v5.5 and v5.7, and the v2 ioctls
> > > added in v5.10, and a minor update for v5.11.
> > > 
> > > The first patch adds a helper method used to print arrays of uint8,
> > > as found in the GPIO API.
> > > The second patch adds the decoding of the GPIO ioctls.
> > > The third patch adds the decoding of the GPIO v2 ioctls.
> > > The fourth patch adds decoding of the
> > > GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME flag which is on track to be added
> > > in Linux v5.11.
> > > 
> > > Kent Gibson (4):
> > >   Add print_uint8_array_member() helper
> > >   Add GPIO ioctl decoding
> > >   Add GPIO uAPI v2 ioctl decoding
> > >   Add GPIO uAPI realtime clock flag
> > > 
> > >  Makefile.am                   |   1 +
> > >  configure.ac                  |   1 +
> > >  defs.h                        |   4 +
> > >  gpio_ioctl.c                  | 536 ++++++++++++++++++++++++++++++++++
> > >  ioctl.c                       |   4 +
> > >  util.c                        |  18 ++
> > >  xlat/gpio_event_flags.in      |   3 +
> > >  xlat/gpio_handle_flags.in     |   8 +
> > >  xlat/gpio_line_flags.in       |   8 +
> > >  xlat/gpio_v2_line_attr_ids.in |   5 +
> > >  xlat/gpio_v2_line_flags.in    |  14 +
> > >  11 files changed, 602 insertions(+)
> > >  create mode 100644 gpio_ioctl.c
> > >  create mode 100644 xlat/gpio_event_flags.in
> > >  create mode 100644 xlat/gpio_handle_flags.in
> > >  create mode 100644 xlat/gpio_line_flags.in
> > >  create mode 100644 xlat/gpio_v2_line_attr_ids.in
> > >  create mode 100644 xlat/gpio_v2_line_flags.in
> > 
> > Thanks for the patch set.  Unfortunately, I don't see any changes
> > to the test suite in this series.  How did you test the new code?
> 
> Yeah, sorry - not sure how to integrate my tests into your test suite,
> and I didn't see anything in the commit requirements mentioning tests
> either.

There is a point of view that explicit requirements of tests would scare
some potential contributors off, although we might reconsider this in the
future.

We strive to have a decent test coverage of strace decoders, currently we
are at 87.5% lines coverage.

strace can be configured using --enable-code-coverage, this allows to
capture the coverage using "make code-coverage-capture" after "make
check".  Please note that strace configured for capturing coverage
is not intended for regular use.

> I'm open to suggestions as to what you would like to see from some tests
> for this.  Testing the whole API is a bear that is probably best left
> out of strace itself.  Even smoke testing a handful of the ioctls in the
> most trivial way is non-trivial to setup - you obviously don't want to
> go messing with real hardware so you need to setup mocks which requires
> kernel modules that you may not have available etc.
> 
> Unless there is some other way to inject ioctls for strace to decode?

strace test suite is run unprivileged and is not expected to have access
to anything non-trivial.
The idea is to test decoders, not the kernel, although we happen to bump
into some kernel corner cases from time to time.
Yes, we use syscall tampering features of strace in some tests.

There is a relatively recent commit v5.6~27 that introduced a comprehensive
set of HDIO_* ioctl decoding tests, you could use it as an example.


-- 
ldv