[PATCH v3 2/6] Add GPIO ioctl decoding

Kent Gibson warthog618 at gmail.com
Sat Jan 23 00:52:39 UTC 2021


On Sat, Jan 23, 2021 at 03:33:12AM +0300, Dmitry V. Levin wrote:
> On Sat, Jan 23, 2021 at 08:04:01AM +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.
> [...]
> > +static int
> > +print_gpiohandle_request(struct tcb *const tcp, const kernel_ulong_t arg)
> > +{
> > +	struct gpiohandle_request hr;
> > +
> > +	if (entering(tcp))
> > +		tprints(", ");
> > +	else if (syserror(tcp))
> > +		return RVAL_IOCTL_DECODED;
> > +	else
> > +		tprints(" => ");
> > +
> > +	if (umove_or_printaddr(tcp, arg, &hr))
> > +		return RVAL_IOCTL_DECODED;
> > +
> > +	if (!entering(tcp)) {
> > +		/* exiting */
> 
> There is a macro called exiting() which means !entering(),
> it can be used here and in other similar cases.
> 

Yeah, there would be wouldn't there.

> > +		PRINT_FIELD_FD("{", hr, fd, tcp);
> > +		tprints("}");
> > +		return RVAL_IOCTL_DECODED;
> > +	}
> > +
> > +	/* entering */
> > +	PRINT_FIELD_U("{", hr, lines);
> > +	if (hr.lines > GPIOHANDLES_MAX)
> > +		hr.lines = GPIOHANDLES_MAX;
> > +	PRINT_FIELD_ARRAY_UPTO(", ", hr, lineoffsets, hr.lines, tcp,
> > +			       print_uint32_array_member);
> > +	PRINT_FIELD_FLAGS(", ", hr, flags, gpio_handle_flags,
> > +			  "GPIOHANDLE_REQUEST_???");
> > +	PRINT_FIELD_ARRAY_UPTO(", ", hr, default_values, hr.lines, tcp,
> > +			       print_uint8_array_member);
> > +	PRINT_FIELD_CSTRING(", ", hr, consumer_label);
> > +	tprints("}");
> 
> PRINT_FIELD_ARRAY_UPTO already does the clamping, see the definition of
> print_local_array_upto(), so hr.lines doesn't require additional checks.
> This applies to all uses of PRINT_FIELD_ARRAY_UPTO.
> 

Good to know.

> Also, hr.lines is printed before other fields, while in the structure it's
> one before the last.  It's usually a good idea to print fields in the
> order they follow in their structure.
> 

I don't put much stock in field order in structs - they are frequently
ordered for packing reasons.
Decoding the size of an array before the array, or multiple arrays in
this case, makes more sense to me.
Having said that, I tend to stick with field order where there is no
good reason to change it. Here there is.

Cheers,
Kent.


More information about the Strace-devel mailing list