diff mbox

v4l2-dbg: report fail reason to the user

Message ID 4AFBB7FD.5090607@freemail.hu (mailing list archive)
State Superseded, archived
Headers

Commit Message

Németh Márton Nov. 12, 2009, 7:23 a.m. UTC 
  From: Márton Németh <nm127@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 <nm127@freemail.hu>
---
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Hans Verkuil Nov. 12, 2009, 7:29 a.m. UTC | #1 
On Thursday 12 November 2009 08:23:41 Németh Márton wrote:
> From: Márton Németh <nm127@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 <nm127@freemail.hu>
> ---
> diff -r 60f784aa071d v4l2-apps/util/v4l2-dbg.cpp
> --- a/v4l2-apps/util/v4l2-dbg.cpp	Wed Nov 11 18:28:53 2009 +0100
> +++ b/v4l2-apps/util/v4l2-dbg.cpp	Thu Nov 12 08:21:20 2009 +0100
> @@ -353,14 +353,21 @@
>  static int doioctl(int fd, int request, void *parm, const char *name)
>  {
>  	int retVal;
> +	int ioctl_errno;
> 
>  	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]) {
> +		/* Save errno because printf() may modify it */
> +		ioctl_errno = errno;
> +		printf("%s: ", name);
> +		if (retVal < 0)
> +			printf("failed: %s\n", strerror(errno));
> +		else
> +			printf("ok\n");

I'm an idiot for not realizing this when I did the first review, but this
can be done without making a copy of errno. Just do this:

	if (options[OptVerbose]) {
		if (retVal < 0)
			printf("%s: failed: %s\n", name, strerror(errno));
		else
			printf("%s: ok\n", name);

Much simpler :-)

Can you change this and post again? Then I'll add it to my pending pull
request.

Thanks,

	Hans

> +		/* Restore errno for caller's use */
> +		errno = ioctl_errno;
> +	}
> 
>  	return retVal;
>  }
> @@ -586,8 +593,8 @@
> 
>  				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",
> +					set_reg.reg, set_reg.val, strerror(errno));
>  			}
>  			set_reg.reg++;
>  		}
>
diff mbox

Patch

diff -r 60f784aa071d v4l2-apps/util/v4l2-dbg.cpp
--- a/v4l2-apps/util/v4l2-dbg.cpp	Wed Nov 11 18:28:53 2009 +0100
+++ b/v4l2-apps/util/v4l2-dbg.cpp	Thu Nov 12 08:21:20 2009 +0100
@@ -353,14 +353,21 @@ 
 static int doioctl(int fd, int request, void *parm, const char *name)
 {
 	int retVal;
+	int ioctl_errno;

 	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]) {
+		/* Save errno because printf() may modify it */
+		ioctl_errno = errno;
+		printf("%s: ", name);
+		if (retVal < 0)
+			printf("failed: %s\n", strerror(errno));
+		else
+			printf("ok\n");
+		/* Restore errno for caller's use */
+		errno = ioctl_errno;
+	}

 	return retVal;
 }
@@ -586,8 +593,8 @@ 

 				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",
+					set_reg.reg, set_reg.val, strerror(errno));
 			}
 			set_reg.reg++;
 		}