[PATCH 2/4] Add GPIO ioctl decoding

[Dmitry V. Levin]

On Wed, Dec 30, 2020 at 10:58:14AM +0800, Kent Gibson wrote:
> On Wed, Dec 30, 2020 at 05:07:42AM +0300, Dmitry V. Levin wrote:
> > On Wed, Dec 23, 2020 at 06:07:46AM +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>
> > [...]
> 
> [...]
> > > +	PRINT_FIELD_U("{", info, line_offset);
> > > +	if (!tcp->u_rval) {
> > > +		tprints(", flags=");
> > > +		printflags(gpio_line_flags, info.flags, "GPIOLINE_FLAG_???");
> > > +		PRINT_FIELD_CSTRING(", ", info, name);
> > > +		PRINT_FIELD_CSTRING(", ", info, consumer);
> > > +	}
> > > +	tprints("}");
> > 
> > Both GPIO_GET_LINEINFO_IOCTL and GPIO_GET_LINEINFO_WATCH_IOCTL are
> > read-write ioctls, that is, the kernel reads struct gpioline_info on
> > entering syscall and writes it back on exiting.  A comprehensive decoder
> > would do the same, i.e. print the relevant parts of the structure both on
> > entering and on exiting.  There would be no need to use
> > umove_or_printaddr_ignore_syserror and ignore syscall errors.
> > 
> 
> Sorry I don't have prompt replies for your other comments - this code has
> been sitting in my working tree for months as I was having problems
> mailing to the list back then so I'll have to spend some time recapturing
> what I was thinking while writing it.
> 
> A decoder can't be comprehensive if it is only decoding relevant parts ;-).
> 
> So should the decoder decode the full struct both ways, or just the fields
> the kernel reads on the way in and the fields it wrote on the way out?

There is no need to print the full struct two times, only those fields
that could be accessed by the kernel.

> I think the policy I went with was not decoding the request, and only
> decoding the response, if the relevant request fields were trivial,
> particularly if the alternative is a full decoding as the vast majority
> of the fields are not relevant.
> It should be just the fields the kernel reads from the request, and
> all fields that the kernel may have written in the response?

Yes, I think so.

> What about fields that contain zeroed values that the kernel interprets
> as default e.g. the event_buffer_size in the v2.  Can they be omitted if
> zero, or should they always be decoded?

I suppose the decoder would be a bit simpler if they are always decoded,
but I don't have a strong preference, that's up to you.


-- 
ldv