[PATCH v2] v4l2(GSOC 2017): added printings for all buf types

[Dmitry V. Levin]

Hi,

On Sun, Mar 26, 2017 at 12:41:50PM +0300, Edgar Kaziahmedov wrote:
> Meaningful and informative printings have been added for all
> v4l2 buf types in the 'print_v4l2_format_fmt' routine.

Our style of commit messages assumes present tense.

> As a new kernel version, it has been added additional
> defines to xlat/v4l2*.in files.

Updates of V4L2 constants should go in a separate commit.

> Apart from this, I want to participate in the GSoC program, in
> particular your project.

Thanks for showing interest in strace GSoC projects.

> --- a/v4l2.c
> +++ b/v4l2.c
> @@ -200,6 +200,8 @@ print_v4l2_fmtdesc(struct tcb *const tcp, const kernel_ulong_t arg)
>  
>  #include "xlat/v4l2_fields.h"
>  #include "xlat/v4l2_colorspaces.h"
> +#include "xlat/v4l2_vbi_flags.h"
> +#include "xlat/v4l2_sliced_flags.h"
>  
>  static void
>  print_v4l2_format_fmt(const char *prefix, const struct_v4l2_format *f)
> @@ -248,37 +250,81 @@ print_v4l2_format_fmt(const char *prefix, const struct_v4l2_format *f)
>  		break;
>  	}
>  #endif
> -
> -	/* TODO: Complete this switch statement */
> -#if 0
>  	case V4L2_BUF_TYPE_VIDEO_OVERLAY:
>  #if HAVE_DECL_V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY
> -	case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY:
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY: {
>  #endif

This is problematic: the opening curly bracket is #if'ed, but the
corresponding closing bracket isn't.

> +                unsigned int i;
> +
>  		tprints(prefix);
> -		tprints("fmt.win={???}");
> +		tprintf("fmt.win={left=%d, top=%d, width=%u, height=%u,field=",

A space after comma is lost.

> +                        ARGS_RECT(f->fmt.win.w));
> +                printxval(v4l2_fields, f->fmt.win.field, "V4L2_FIELD_???");

Something went wrong with the indentation.  At least in this file
indentation is implemented using TABs, please follow the style.

> +                tprintf(", chromakey=0x%08x, clips=[", f->fmt.win.chromakey);
> +                for (i = 0; i < f->fmt.win.clipcount; i++) {

v4l2_window.clipcount is provided by tracee and therefore cannot be trusted.

> +                        if (i > 0)
> +                                tprints(", ");
> +                        tprintf(FMT_RECT, ARGS_RECT(f->fmt.win.clips->c));
> +                }

v4l2_window.clips is a tracee's pointer to its memory.
Try to run this code, the segfault is already waiting for you. :)

We have primitives for printing tracee's arrays, please use them instead.

> +                tprintf("], clipcount=%u, bitmap=%p, global_alpha=0x%02x}",

v4l2_window.bitmap is a tracee's pointer, use printaddr.
Also, use %#x instead of custom formats like 0x%02x.

OK, that's enough for now.  Please do not forget to add a test case
for this new functionality.


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