[PATCH 2/2] v4l2: decode VIDIOC_QUERY_EXT_CTRL:arg

[Eugene Syromiatnikov]

On Tue, Feb 11, 2020 at 09:33:57AM +0100, Philippe De Muyter wrote:
> * v4l2.c: add print_v4l2_query_ext_ctrl, and use it for
> VIDIOC_QUERY_EXT_CTRL:arg
> 
> Signed-off-by: Philippe De Muyter <phdm at macqel.be>

Thank you for contributing the original v4l2 decoder.

> ---
>  v4l2.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/v4l2.c b/v4l2.c
> index afa5d44..566a4aa 100644
> --- a/v4l2.c
> +++ b/v4l2.c

> +static void
> +print_v4l2_query_ctrl_id(unsigned long id)
> +{
> +	if (id & V4L2_CTRL_FLAG_NEXT_CTRL) {
> +		tprints("V4L2_CTRL_FLAG_NEXT_CTRL|");
> +		id &= ~V4L2_CTRL_FLAG_NEXT_CTRL;
> +	}
> +	if (id & V4L2_CTRL_FLAG_NEXT_COMPOUND) {
> +		tprints("V4L2_CTRL_FLAG_NEXT_COMPOUND|");
> +		id &= ~V4L2_CTRL_FLAG_NEXT_COMPOUND;
> +	}
> +	printxval(v4l2_control_ids, id, NULL);
> +}

There's already print_v4l2_cid function that does better job at control
ID decoding, but indeed lacks V4L2_CTRL_FLAG_NEXT_* decoding
capabilities.

> +
> +static int
> +print_v4l2_query_ext_ctrl(struct tcb *const tcp, const kernel_ulong_t arg)
> +{
> +	struct v4l2_query_ext_ctrl c;

It won't build the systems with pre-3.17 headers, as they don't have this
type definition.

> +		if (!(c.id & NEXT_FLAGS) || !abbrev(tcp)) {
> +			tprints("=");
> +			print_v4l2_query_ctrl_id(c.id);
> +		}
> +	} else {
> +		unsigned long entry_id = get_tcb_priv_ulong(tcp);
> +
> +		if (syserror(tcp) || umove(tcp, arg, &c) < 0) {
> +			if (abbrev(tcp) && (entry_id & NEXT_FLAGS)) {
> +				tprints("=");
> +				print_v4l2_query_ctrl_id(entry_id);
> +			}
> +			tprints("}");
> +			return RVAL_IOCTL_DECODED;
> +		}
> +		if (entry_id & NEXT_FLAGS) {
> +			tprints(" => ");
> +			print_v4l2_query_ctrl_id(c.id);
> +		}

Why is this dance with abbrev needed? Why not simply print entering
control ID on entering and then amend it with " => <new_id>" printing on
exiting?

> +	}
> +#undef NEXT_FLAGS
> +
> +	if (exiting(tcp)) {
> +		tprints(", type=");
> +		printxval(v4l2_control_types, c.type, "V4L2_CTRL_TYPE_???");
> +		PRINT_FIELD_CSTRING(", ", c, name);
> +		if (!abbrev(tcp)) {
> +			tprintf(", minimum=%lld, maximum=%lld, step=%lld"

step is unsigned in struct v4l2_query_ext_ctrl.

Also, it won't build on mips, power, or alpha, as these architecture
define __u64/__s64 differently.

Why the elem_size, elems, nr_of_dims, dims, and reserved fields
are not decoded?

> +	case VIDIOC_QUERY_EXT_CTRL: /* RW */

It won't build on (pre-3.17) systems that don't have this definition.

Please take a look at the proposed decoder updates (in the attachment,
to be applied along with the original patch on top of esyr/dts-fixes branch,
available in strace's git mirrors).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-v4l2-VIDIOC_QUERY_EXT_CTRL-decoder-updates.patch
Type: text/x-diff
Size: 10362 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20200304/220d8c69/attachment.bin>