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
pgpPn8H6yJcut.pgp
Description: PGP signature
------------------------------------------------------------------------------ HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions Find What Matters Most in Your Big Data with HPCC Systems Open Source. Fast. Scalable. Simple. Ideal for Dirty Data. Leverages Graph Analysis for Fast Processing & Easy Data Exploration http://p.sf.net/sfu/hpccsystems
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
