[PATCH 3/4] Add GPIO uAPI v2 ioctl decoding

[Kent Gibson]

On Sat, Jan 02, 2021 at 07:02:51PM +0300, Dmitry V. Levin wrote:
> On Sat, Jan 02, 2021 at 11:53:22PM +0800, Kent Gibson wrote:
> > On Wed, Dec 30, 2020 at 05:17:58AM +0300, Dmitry V. Levin wrote:
> > > On Wed, Dec 23, 2020 at 06:07:47AM +0800, Kent Gibson wrote:
> > > > Add decoding of GPIO uAPI v2 ioctls added in Linux v5.10.
> > > [...]
> > > > +#include "xlat/gpio_v2_line_attr_ids.h"
> > > > +
> > > > +static void
> > > > +print_gpio_v2_line_attribute(struct tcb *const tcp,
> > > > +			     struct gpio_v2_line_attribute *attr)
> > > > +{
> > > > +	switch (attr->id) {
> > > 
> > > Please also print attr->id using gpio_v2_line_attr_ids.
> > 
> > But why?  The id is only present to indicate which field in the union to
> > decode.
> > If it is one of the gpio_v2_line_attr_ids then it decodes to the
> > appropriate field=value.
> > If it is not a gpio_v2_line_attr_ids value then it decodes raw.
> 
> Do you mean a valid attr->id would be redundant in the output?
> 

Correct.  It is effectively encoded in the field name.
So the user can currently pass one of three possible attributes - flags,
values or debounce_period_us.  The raw case is there to catch decoding
future attributes that this decoder doesn't know about.

> > > > +	case GPIO_V2_LINE_ATTR_ID_FLAGS:
> > > > +		PRINT_FIELD_FLAGS("", *attr, flags, gpio_v2_line_flags,
> > > > +				  "GPIO_V2_LINE_FLAG_???");
> > > > +		break;
> > > > +	case GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES:
> > > > +		PRINT_FIELD_X("", *attr, values);
> > > > +		break;
> > > > +	case GPIO_V2_LINE_ATTR_ID_DEBOUNCE:
> > > > +		PRINT_FIELD_U("", *attr, debounce_period_us);
> > > > +		break;
> > > > +	default:
> > > > +		tprintf("(id=%u)=%#" PRI__x64, attr->id, attr->values);
> > > 
> > > I suppose GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES case could be used to handle
> > > this case, too.
> > 
> > This is the raw case - you don't have a field name as the id is not one of
> > the gpio_v2_line_attr_ids, hence the special treatment.
> 
> If it's a raw case, why not just print it using PRINT_FIELD_X("", *attr, id)?
> 
> 

Cos in the raw case you need to print the id AND the content of the
union raw to display the complete content.
PRINT_FIELD_X("", *attr, id) would print the id, but not the union
value.  I would need to add PRINT_FIELD_X("", *attr, values) as well.
(the value covers the full content of the union)

The output would then look like "id=6, values=0x12345678", but my concern
was that casual viewers might be misled that the id was
GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES, and the values field was the intended
attribute being passed.

So I went with the "(id=6)=0x12345678" decoding instead.
If enyone ever sees that then it means they need to update to the latest
strace that can decode that particular id into the correct field name,
and the value to the appropriate format.

Cheers,
Kent.