diff mbox

[2/4,media] usbvision-core: Use common error handling code in usbvision_set_compress_params()

Message ID 52c09836-83d7-c509-6e85-c7af16160302@users.sourceforge.net (mailing list archive)
State New, archived
Headers show

Commit Message

SF Markus Elfring Sept. 21, 2017, 3:07 p.m. UTC 
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 21 Sep 2017 12:45:49 +0200

* Add a jump target so that a bit of exception handling can be better
  reused at the end of this function.

* Replace the local variable "proc" by the identifier "__func__".

* Use the interface "dev_err" instead of "printk".

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/usbvision/usbvision-core.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

Comments

Dan Carpenter Sept. 22, 2017, 11:44 a.m. UTC | #1 
On Thu, Sep 21, 2017 at 05:07:06PM +0200, SF Markus Elfring wrote:
> @@ -1913,11 +1908,12 @@ static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
>  			     USB_DIR_OUT | USB_TYPE_VENDOR |
>  			     USB_RECIP_ENDPOINT, 0,
>  			     (__u16) USBVISION_PCM_THR1, value, 6, HZ);
> +	if (rc < 0)
> +report_failure:
> +		dev_err(&usbvision->dev->dev,
> +			"%s: ERROR=%d. USBVISION stopped - reconnect or reload driver.\n",
> +			__func__, rc);

You've been asked several times not to write code like this.  You do
it later in the patch series as well.

regards,
dan carpenter
SF Markus Elfring Sept. 22, 2017, 2:50 p.m. UTC | #2 
>> @@ -1913,11 +1908,12 @@ static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
>>  			     USB_DIR_OUT | USB_TYPE_VENDOR |
>>  			     USB_RECIP_ENDPOINT, 0,
>>  			     (__u16) USBVISION_PCM_THR1, value, 6, HZ);
>> +	if (rc < 0)
>> +report_failure:
>> +		dev_err(&usbvision->dev->dev,
>> +			"%s: ERROR=%d. USBVISION stopped - reconnect or reload driver.\n",
>> +			__func__, rc);
> 
> You've been asked several times not to write code like this.

This suggestion occurred a few times.

Do you prefer to move this place to the end together with a duplicated statement “return rc;”?


> You do it later in the patch series as well.

To which update step do you refer here?

Regards,
Markus
diff mbox

Patch

diff --git a/drivers/media/usb/usbvision/usbvision-core.c b/drivers/media/usb/usbvision/usbvision-core.c
index 16b76c85eeec..bb6f4f69165f 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1857,7 +1857,6 @@  int usbvision_stream_interrupt(struct usb_usbvision *usbvision)
 
 static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
 {
-	static const char proc[] = "usbvision_set_compresion_params: ";
 	int rc;
 	unsigned char *value = usbvision->ctrl_urb_buffer;
 
@@ -1882,12 +1881,8 @@  static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
 			     USB_DIR_OUT | USB_TYPE_VENDOR |
 			     USB_RECIP_ENDPOINT, 0,
 			     (__u16) USBVISION_INTRA_CYC, value, 5, HZ);
-
-	if (rc < 0) {
-		printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or reload driver.\n",
-		       proc, rc);
-		return rc;
-	}
+	if (rc < 0)
+		goto report_failure;
 
 	if (usbvision->bridge_type == BRIDGE_NT1004) {
 		value[0] =  20; /* PCM Threshold 1 */
@@ -1913,11 +1908,12 @@  static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
 			     USB_DIR_OUT | USB_TYPE_VENDOR |
 			     USB_RECIP_ENDPOINT, 0,
 			     (__u16) USBVISION_PCM_THR1, value, 6, HZ);
+	if (rc < 0)
+report_failure:
+		dev_err(&usbvision->dev->dev,
+			"%s: ERROR=%d. USBVISION stopped - reconnect or reload driver.\n",
+			__func__, rc);
 
-	if (rc < 0) {
-		printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or reload driver.\n",
-		       proc, rc);
-	}
 	return rc;
 }