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

Dmitry V. Levin ldv at altlinux.org
Sat Jan 2 16:02:51 UTC 2021


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?

> > > +	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)?


-- 
ldv


More information about the Strace-devel mailing list