[PATCH v2] v4l2.c: a new (incomplete) decoder for Video4Linux ioctls args

[Dmitry V. Levin]

On Tue, Jun 03, 2014 at 11:29:42AM +0200, Philippe De Muyter wrote:
> --- /dev/null
> +++ b/v4l2.c
[...]
> +#ifdef linux

Starting with strace v4.7 it is always linux.

[...]
> +#include "xlat/v4l2_device_capabilities_flags.h"
> +
> +#include "xlat/v4l2_buf_types.h"
> +
> +#include "xlat/v4l2_buf_flags.h"
> +
> +#include "xlat/v4l2_framesize_types.h"
> +
> +#include "xlat/v4l2_frameinterval_types.h"
> +
> +#include "xlat/v4l2_fields.h"
> +
> +#include "xlat/v4l2_colorspaces.h"
> +
> +#include "xlat/v4l2_format_description_flags.h"
> +
> +#include "xlat/v4l2_memories.h"
> +
> +#include "xlat/v4l2_control_ids.h"
> +
> +#include "xlat/v4l2_control_types.h"
> +
> +#include "xlat/v4l2_control_flags.h"
> +
> +#include "xlat/v4l2_control_classes.h"
> +
> +#include "xlat/v4l2_streaming_capabilities.h"
> +
> +#include "xlat/v4l2_capture_modes.h"
> +
> +#include "xlat/v4l2_input_types.h"

These empty lines are not necessary.

[...]
> +#if (WORDS_BIGENDIAN == 1)

I suggest a simpler form: #if WORDS_BIGENDIAN

> +		tprintf("pix={width=%u, height=%u, pixelformat=",
> +						pix->width, pix->height);

Please fix indentation in this and other similar cases.

[...]
> +#if defined(HAVE_DECL_V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) && HAVE_DECL_V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE

All macros listed in AC_CHECK_DECLS are defined.

> +	case VIDIOC_G_FMT:
> +	case VIDIOC_S_FMT:
> +	case VIDIOC_TRY_FMT: {
> +		struct v4l2_format f;
> +
> +		if (umove(tcp, arg, &f) < 0) {
> +			tprintf(", %#lx", arg);
> +		} else {
> +			if (entering(tcp)) {
> +				tprints(", {type=");
> +				printxval(v4l2_buf_types, f.type, "V4L2_BUF_TYPE_???");
> +			}
> +			if ((entering(tcp) && code != VIDIOC_G_FMT)
> +			|| (exiting(tcp) && !syserror(tcp))) {
> +				tprints(exiting(tcp) ? " => " : ", ");
> +				print_v4l2_format_fmt(&f);
> +			}
> +			if (exiting(tcp))
> +				tprints("}");
> +		}
> +		return 1;

In this and other similar places, no need to repeat the code from default
ioctl parser.  Please replace
	if (umove(tcp, arg, &f) < 0) {
		tprintf(", %#lx", arg);
	} else {
		...
	}
with
	if (umove(tcp, arg, &f) < 0)
		return 0;
	...

> +	case VIDIOC_G_PARM:
> +	case VIDIOC_S_PARM: {
> +		struct v4l2_streamparm s;
> +
> +		if (entering(tcp))
> +			return 1;
> +		if (umove(tcp, arg, &s) < 0) {
> +			tprintf(", %#lx", arg);
> +		} else {
> +			tprints(", {type=");
> +			printxval(v4l2_buf_types, s.type, "V4L2_BUF_TYPE_???");
> +
> +			if (s.type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {

In this and many other similar cases, please ensure that
you are not printing too much garbage in case of syserror(tcp).

[...]
> +	case VIDIOC_S_EXT_CTRLS:
> +	case VIDIOC_TRY_EXT_CTRLS:
> +	case VIDIOC_G_EXT_CTRLS: {
> +		struct v4l2_ext_controls c;
> +		unsigned n;
> +
> +		if (entering(tcp) || umove(tcp, arg, &c) < 0)
> +			return 0;
> +
> +		tprints(", ctrl_class=");
> +		printxval(v4l2_control_classes, c.ctrl_class,
> +			  "V4L2_CTRL_CLASS_???");
> +		tprintf(", count=%u, error_idx=%u, controls=[", c.count,
> +			c.error_idx);
> +
> +		for (n = 0; n < c.count; ++n) {

c.count is uint32_t, so please ensure that you are not going to print
2^32-1 elements, especially if the structure does not come from the
kernel.

> +			struct v4l2_ext_control ctrl;
> +			if (n > 0)
> +				tprints(", ");
> +			umove(tcp, (long) (c.controls + n), &ctrl);

Please check umove return code; there is no need to fetch remaining
elements if umove fails.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20140614/c3d29298/attachment.bin>