[PATCH v4 2/6] Add GPIO ioctl decoding

[Dmitry V. Levin]

On Mon, Jan 25, 2021 at 08:34:56AM +0800, Kent Gibson wrote:
> On Sun, Jan 24, 2021 at 06:06:42PM +0300, Dmitry V. Levin wrote:
> > On Sat, Jan 23, 2021 at 09:56:41AM +0800, Kent Gibson wrote:
> > > Decode the GPIO character device ioctls first introduced in Linux v4.8,
> > > as well as those added in subsequent kernel releases up to Linux v5.7.
> > > 
> > > Signed-off-by: Kent Gibson <warthog618 at gmail.com>
> > 
> > I've given this a wider testing and found some portability issues.
> > 
> > First of all, include/uapi/linux/gpio.h was first introduced a bit
> > earlier: in Linux v4.6 it contained just 2 ioctl commands
> > (GPIO_GET_CHIPINFO_IOCTL and GPIO_GET_LINEINFO_IOCTL) and just two
> > definitions of structures (gpiochip_info and gpioline_info), so this code
> > doesn't compile with kernel headers from Linux v4.6 and v4.7.
> 
> Sorry - didn't realise those preliminary versions of gpio.h made it into
> the released kernel.  They actually define a different ioctl major, 'o'
> instead of 0xb4, that conflicted with other ioctls.

Fortunately, no: the header was introduced by commit
v4.6-rc1~108^2~104^2~3, and 'o' was changed later to 0xB4 by commit
v4.6-rc1~108^2~6, so there were no released kernels with 'o'.

> So probably best to not use those versions of gpio.h from the kernel.
> That rules out using HAVE_LINUX_GPIO_H.
> 
> > I see 3 alternative fixes for this portability issue:
> > - check for structures available since v4.8 and bump requirements
> >   appropriately;
> > - add to types/gpio.h definitions of those 4 structures that were
> >   introduced in Linux v4.8;
> > - add to types/gpio.h definitions of all 6 structures defined since Linux
> >   v4.8 and lift the HAVE_LINUX_GPIO_H requirement altogether.
> > 
> > I'm inclined to choose the last alternative, see the proposed patch below.
> 
> Of the three options, I agree that the third is the best - only use the
> kernel gpio.h to check that the local type definitions match.
> 
> Is there a reason you couldn't just include the latest kernel gpio.h in
> the strace build directly, rather than having to redefine everything in
> types/gpio.h?

Yes, this is certainly possible, in fact, this is what I suggest to do in
the test, but we need fallback definitions for older kernel headers, too,
so there should be no material difference.

Also, we don't have automation for that redirection back to linux/gpio.h,
and a multitude of #ifdef HAVE_STRUCT_GPIO*/#define/#endif doesn't look
that nice.

I must admit that linux/gpio.h is good enough to allow any of these two
approaches, there were no incompatible changes once it was included into
released kernels.  Unfortunately, not all uapi headers are good enough in
this respect.


-- 
ldv