staging: comedi: usbduxsigma: Improve unlocking of a mutex in three functions
Subsystems (2)
PATCH 1/1 staging: comedi: usbduxsigma: Improve unlocking of a mutex in three functions
From: Markus Elfring <[email protected]>
Date: Thu, 2 Nov 2017 21:16:50 +0100
* Add a jump target so that a call of the function "mutex_unlock" is stored only twice in these function implementations.
* Replace seven calls by goto statements.
This issue was detected by using the Coccinelle software.
--- drivers/staging/comedi/drivers/usbduxsigma.c | 48 +++++++++++++++------------- 1 file changed, 26 insertions(+), 22 deletions(-)
drivers/staging/comedi/drivers/usbduxsigma.c +26 -22
@@ -672,10 +672,8 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev, devpriv->dux_commands[8 ] = sysred; ret =usbbuxsigma_send_cmd (dev,USBBUXSIGMA_AD_CMD ); -if (ret <0 ) { -mutex_unlock (&devpriv->mut); -return ret; - } +if (ret <0 ) + goto unlock; devpriv->ai_counter = devpriv->ai_timer;
@@ -686,8 +684,7 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev, devpriv->n_ai_urbs, 1);if (ret <0 ) { devpriv->ai_cmd_running =0 ; -mutex_unlock (&devpriv->mut); -return ret; + goto unlock; } s->async->inttrig =NULL ; }else {/* TRIG_INT */
@@ -697,6 +694,10 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,mutex_unlock (&devpriv->mut);return 0 ; + +unlock: +mutex_unlock (&devpriv->mut); +return ret; } static int usbduxsigma_ai_insn_read(struct comedi_device *dev,
@@ -714,8 +715,8 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,mutex_lock (&devpriv->mut);if (devpriv->ai_cmd_running) { -mutex_unlock (&devpriv->mut); -return -EBUSY ; + ret = -EBUSY ; + goto unlock; }create_adc_command (chan, &muxsg0, &muxsg1);
@@ -730,19 +731,15 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,/* adc commands */ ret =usbbuxsigma_send_cmd (dev,USBDUXSIGMA_SINGLE_AD_CMD ); -if (ret <0 ) { -mutex_unlock (&devpriv->mut); -return ret; - } +if (ret <0 ) + goto unlock;for (i =0 ; i < insn->n; i++) {u32 val; ret =usbduxsigma_receive_cmd (dev,USBDUXSIGMA_SINGLE_AD_CMD ); -if (ret <0 ) { -mutex_unlock (&devpriv->mut); -return ret; - } +if (ret <0 ) + goto unlock;/* 32 bits big endian from the A/D converter */ val = be32_to_cpu(get_unaligned((__be32
@@ -753,6 +750,10 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,mutex_unlock (&devpriv->mut);return insn->n; + +unlock: +mutex_unlock (&devpriv->mut); +return ret; } static int usbduxsigma_ao_insn_read(struct comedi_device *dev,
@@ -782,8 +783,8 @@ static int usbduxsigma_ao_insn_write(struct comedi_device *dev,mutex_lock (&devpriv->mut);if (devpriv->ao_cmd_running) { -mutex_unlock (&devpriv->mut); -return -EBUSY ; + ret = -EBUSY ; + goto unlock; }for (i =0 ; i < insn->n; i++) {
@@ -791,15 +792,18 @@ static int usbduxsigma_ao_insn_write(struct comedi_device *dev, devpriv->dux_commands[2 ] = data[i];/* value */ devpriv->dux_commands[3 ] = chan;/* channel number */ ret =usbbuxsigma_send_cmd (dev,USBDUXSIGMA_DA_CMD ); -if (ret <0 ) { -mutex_unlock (&devpriv->mut); -return ret; - } +if (ret <0 ) + goto unlock; + s->readback[chan] = data[i]; }mutex_unlock (&devpriv->mut);return insn->n; + +unlock: +mutex_unlock (&devpriv->mut); +return ret; } static int usbduxsigma_ao_inttrig(struct comedi_device *dev,
Signature · 1 line
2.15.0
Quoted 43 lines · SF Markus Elfring
From: Markus Elfring <[email protected]>
Date: Thu, 2 Nov 2017 21:16:50 +0100* Add a jump target so that a call of the function "
mutex_unlock" is stored only twice in these function implementations.* Replace seven calls by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by Markus Elfring <[email protected]>--- drivers/staging/comedi/drivers/usbduxsigma.c | 48 +++++++++++++++------------- 1 file changed, 26 insertions(+), 22 deletions(-)drivers/staging/comedi/drivers/usbduxsigma.c +3
-6@@ -672,10 +672,8 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev, devpriv->dux_commands[8 ] = sysred; ret =usbbuxsigma_send_cmd (dev,USBBUXSIGMA_AD_CMD ); -if (ret <0 ) { -mutex_unlock (&devpriv->mut); -return ret; - } +if (ret <0 ) + goto unlock; devpriv->ai_counter = devpriv->ai_timer;@@ -686,8 +684,7 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev, devpriv->n_ai_urbs, 1);if (ret <0 ) { devpriv->ai_cmd_running =0 ; -mutex_unlock (&devpriv->mut); -return ret; + goto unlock; } s->async->inttrig =NULL ; }else {/* TRIG_INT */
drivers/staging/comedi/drivers/usbduxsigma.c static int usbduxsigma_ai_cmd(struct comedi_device *dev, · line 696
Quoted 3 lines
drivers/staging/comedi/drivers/usbduxsigma.c +0
-0@@ -697,6 +694,10 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,mutex_unlock (&devpriv->mut);return 0 ;
You could also remove that call by replacing the above three lines with:
ret = 0;
and falling through to the 'unlock:' label. (Actually, you don't even need the above line, as ret should already be 0 at this point, but I think it is easier to understand the code if ret is set to 0 explicitly.)
Quoted 43 lines
drivers/staging/comedi/drivers/usbduxsigma.c +10
-10+ +unlock: +mutex_unlock (&devpriv->mut); +return ret; } static int usbduxsigma_ai_insn_read(struct comedi_device *dev,@@ -714,8 +715,8 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,mutex_lock (&devpriv->mut);if (devpriv->ai_cmd_running) { -mutex_unlock (&devpriv->mut); -return -EBUSY ; + ret = -EBUSY ; + goto unlock; }create_adc_command (chan, &muxsg0, &muxsg1);@@ -730,19 +731,15 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,/* adc commands */ ret =usbbuxsigma_send_cmd (dev,USBDUXSIGMA_SINGLE_AD_CMD ); -if (ret <0 ) { -mutex_unlock (&devpriv->mut); -return ret; - } +if (ret <0 ) + goto unlock;for (i =0 ; i < insn->n; i++) {u32 val; ret =usbduxsigma_receive_cmd (dev,USBDUXSIGMA_SINGLE_AD_CMD ); -if (ret <0 ) { -mutex_unlock (&devpriv->mut); -return ret; - } +if (ret <0 ) + goto unlock;/* 32 bits big endian from the A/D converter */ val = be32_to_cpu(get_unaligned((__be32
drivers/staging/comedi/drivers/usbduxsigma.c static int usbduxsigma_ai_insn_read(struct comedi_device *dev, · line 752
Quoted 3 lines
drivers/staging/comedi/drivers/usbduxsigma.c +0
-0@@ -753,6 +750,10 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,mutex_unlock (&devpriv->mut);return insn->n;
Similarly, you could replace the above three lines with:
ret = insn->n;
and fall through to the label.
Quoted 24 lines
drivers/staging/comedi/drivers/usbduxsigma.c +6
-4+ +unlock: +mutex_unlock (&devpriv->mut); +return ret; } static int usbduxsigma_ao_insn_read(struct comedi_device *dev,@@ -782,8 +783,8 @@ static int usbduxsigma_ao_insn_write(struct comedi_device *dev,mutex_lock (&devpriv->mut);if (devpriv->ao_cmd_running) { -mutex_unlock (&devpriv->mut); -return -EBUSY ; + ret = -EBUSY ; + goto unlock; }for (i =0 ; i < insn->n; i++) {@@ -791,15 +792,18 @@ static int usbduxsigma_ao_insn_write(struct comedi_device *dev, devpriv->dux_commands[2 ] = data[i];/* value */ devpriv->dux_commands[3 ] = chan;/* channel number */ ret =usbbuxsigma_send_cmd (dev,USBDUXSIGMA_DA_CMD ); -if (ret <0 ) { -mutex_unlock (&devpriv->mut);
drivers/staging/comedi/drivers/usbduxsigma.c static int usbduxsigma_ao_insn_write(struct comedi_device *dev, · line 802
Quoted 10 lines
drivers/staging/comedi/drivers/usbduxsigma.c +3
-2-return ret; - } +if (ret <0 ) + goto unlock; + s->readback[chan] = data[i]; }mutex_unlock (&devpriv->mut);return insn->n;
Ditto.
Quoted 8 lines
drivers/staging/comedi/drivers/usbduxsigma.c +4
-0+ +unlock: +mutex_unlock (&devpriv->mut); +return ret; } static int usbduxsigma_ao_inttrig(struct comedi_device *dev,
Signature · 2 lines
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=- -=( Web: http://www.mev.co.uk/ )=-