On Sunday 08 November 2009 13:32:50 Németh Márton wrote:
> From: Márton Németh <nm...@freemail.hu>
> 
> Report the fail reason to the user when writing a register even if
> the verbose mode is switched off.
> 
> Remove duplicated code ioctl() call which may cause different ioctl()
> function call in case of verbose and non verbose if not handled carefully.
> 
> Signed-off-by: Márton Németh <nm...@freemail.hu>
> ---
> diff -r 19c0469c02c3 v4l2-apps/util/v4l2-dbg.cpp
> --- a/v4l2-apps/util/v4l2-dbg.cpp     Sat Nov 07 15:51:01 2009 -0200
> +++ b/v4l2-apps/util/v4l2-dbg.cpp     Sun Nov 08 14:13:52 2009 +0100
> @@ -354,13 +354,14 @@
>  {
>       int retVal;
> 
> -     if (!options[OptVerbose]) return ioctl(fd, request, parm);
>       retVal = ioctl(fd, request, parm);
> -     printf("%s: ", name);
> -     if (retVal < 0)
> -             printf("failed: %s\n", strerror(errno));
> -     else
> -             printf("ok\n");
> +     if (options[OptVerbose]) {
> +             printf("%s: ", name);
> +             if (retVal < 0)
> +                     printf("failed: %s\n", strerror(errno));

Strictly speaking if the printf two line back would produce an error, then
errno would now contain the errno from the printf and not from the ioctl.
So errno should be assigned to a local variable right after the ioctl.

Note that this was already a bug in the original code.

Note also that I fail to see any difference in the old vs the new code.

> +             else
> +                     printf("ok\n");
> +     }
> 
>       return retVal;
>  }
> @@ -586,8 +587,9 @@
> 
>                               printf(" set to 0x%llx\n", set_reg.val);
>                       } else {
> -                             printf("Failed to set register 0x%08llx value 
> 0x%llx\n",
> -                                     set_reg.reg, set_reg.val);
> +                             printf("Failed to set register 0x%08llx value 
> 0x%llx: "
> +                                     "%s\n",

Please keep this printf on one line. Yes, you get a checkpatch warning but
I'd rather have a slightly longer line than an ugly split line.

Regards,

        Hans

> +                                     set_reg.reg, set_reg.val, 
> strerror(errno));
>                       }
>                       set_reg.reg++;
>               }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to