[PATCH] staging: comedi: Compress return logic into one line.

[PATCH] staging: comedi: Compress return logic into one line.

[Varsha Rao]

Simplify function return by merging assignment and return into a single
line. The following coccinelle script is used to fix this issue.

@@
expression e,ret;
@@

-ret = e;
-return ret;
+return e;

Signed-off-by: Varsha Rao <rvarsha016@gmail.com>
---
 drivers/staging/comedi/drivers.c             | 3 +--
 drivers/staging/comedi/drivers/cb_pcidas64.c | 8 ++------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index a5bf2cc..76e399c 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -86,8 +86,7 @@ static void comedi_clear_hw_dev(struct comedi_device *dev)
  */
 void *comedi_alloc_devpriv(struct comedi_device *dev, size_t size)
 {
-	dev->private = kzalloc(size, GFP_KERNEL);
-	return dev->private;
+	return kzalloc(size, GFP_KERNEL);
 }
 EXPORT_SYMBOL_GPL(comedi_alloc_devpriv);
 
diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c b/drivers/staging/comedi/drivers/cb_pcidas64.c
index 3b98193..be65f71 100644
--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
@@ -1394,9 +1394,7 @@ static int set_ai_fifo_segment_length(struct comedi_device *dev,
 	writew(devpriv->fifo_size_bits,
 	       devpriv->main_iobase + FIFO_SIZE_REG);
 
-	devpriv->ai_fifo_segment_length = num_increments * increment_size;
-
-	return devpriv->ai_fifo_segment_length;
+	return num_increments * increment_size;
 }
 
 /*
@@ -1417,9 +1415,7 @@ static int set_ai_fifo_size(struct comedi_device *dev, unsigned int num_samples)
 	if (retval < 0)
 		return retval;
 
-	num_samples = retval * fifo->num_segments * fifo->sample_packing_ratio;
-
-	return num_samples;
+	return retval * fifo->num_segments * fifo->sample_packing_ratio;
 }
 
 /* query length of fifo */
-- 
2.9.3

Re: [Outreachy kernel] [PATCH] staging: comedi: Compress return logic into one line.

[Julia Lawall]

On Sun, 12 Mar 2017, Varsha Rao wrote:

> Simplify function return by merging assignment and return into a single
> line. The following coccinelle script is used to fix this issue.
>
> @@
> expression e,ret;
> @@
>
> -ret = e;
> -return ret;
> +return e;

This is too general.  For example, in your first example, you remove the
initialization of dev->private.  It's fine to remove the initialization of
a local variable that is about to go out of scope, but not to remove the
initialization of a field of a structure that can be accessed after the
end of the function.

julia

>
> Signed-off-by: Varsha Rao <rvarsha016@gmail.com>
> ---
>  drivers/staging/comedi/drivers.c             | 3 +--
>  drivers/staging/comedi/drivers/cb_pcidas64.c | 8 ++------
>  2 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> index a5bf2cc..76e399c 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -86,8 +86,7 @@ static void comedi_clear_hw_dev(struct comedi_device *dev)
>   */
>  void *comedi_alloc_devpriv(struct comedi_device *dev, size_t size)
>  {
> -	dev->private = kzalloc(size, GFP_KERNEL);
> -	return dev->private;
> +	return kzalloc(size, GFP_KERNEL);
>  }
>  EXPORT_SYMBOL_GPL(comedi_alloc_devpriv);
>
> diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c b/drivers/staging/comedi/drivers/cb_pcidas64.c
> index 3b98193..be65f71 100644
> --- a/drivers/staging/comedi/drivers/cb_pcidas64.c
> +++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
> @@ -1394,9 +1394,7 @@ static int set_ai_fifo_segment_length(struct comedi_device *dev,
>  	writew(devpriv->fifo_size_bits,
>  	       devpriv->main_iobase + FIFO_SIZE_REG);
>
> -	devpriv->ai_fifo_segment_length = num_increments * increment_size;
> -
> -	return devpriv->ai_fifo_segment_length;
> +	return num_increments * increment_size;
>  }
>
>  /*
> @@ -1417,9 +1415,7 @@ static int set_ai_fifo_size(struct comedi_device *dev, unsigned int num_samples)
>  	if (retval < 0)
>  		return retval;
>
> -	num_samples = retval * fifo->num_segments * fifo->sample_packing_ratio;
> -
> -	return num_samples;
> +	return retval * fifo->num_segments * fifo->sample_packing_ratio;
>  }
>
>  /* query length of fifo */
> --
> 2.9.3
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/58c53fae.4737620a.4ce3e.692c%40mx.google.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] staging: comedi: Compress return logic into one line.

[Varsha Rao]

[-- Attachment #1.1: Type: text/plain, Size: 802 bytes --]



On Sunday, March 12, 2017 at 6:05:52 PM UTC+5:30, Julia Lawall wrote:
>
>
>
> On Sun, 12 Mar 2017, Varsha Rao wrote: 
>
> > Simplify function return by merging assignment and return into a single 
> > line. The following coccinelle script is used to fix this issue. 
> > 
> > @@ 
> > expression e,ret; 
> > @@ 
> > 
> > -ret = e; 
> > -return ret; 
> > +return e; 
>
> This is too general.  For example, in your first example, you remove the 
> initialization of dev->private.  It's fine to remove the initialization of 
> a local variable that is about to go out of scope, but not to remove the 
> initialization of a field of a structure that can be accessed after the 
> end of the function. 
>
>  
     Yes, you are right. Sorry for it. I will send a new patch.
      
      Thanks,
      Varsha

[-- Attachment #1.2: Type: text/html, Size: 1101 bytes --]

Re: [PATCH] staging: comedi: Compress return logic into one line.

[Greg Kroah-Hartman]

On Sun, Mar 12, 2017 at 06:01:35PM +0530, Varsha Rao wrote:
> Simplify function return by merging assignment and return into a single
> line. The following coccinelle script is used to fix this issue.
> 
> @@
> expression e,ret;
> @@
> 
> -ret = e;
> -return ret;
> +return e;
> 
> Signed-off-by: Varsha Rao <rvarsha016@gmail.com>
> ---
>  drivers/staging/comedi/drivers.c             | 3 +--
>  drivers/staging/comedi/drivers/cb_pcidas64.c | 8 ++------
>  2 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> index a5bf2cc..76e399c 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -86,8 +86,7 @@ static void comedi_clear_hw_dev(struct comedi_device *dev)
>   */
>  void *comedi_alloc_devpriv(struct comedi_device *dev, size_t size)
>  {
> -	dev->private = kzalloc(size, GFP_KERNEL);
> -	return dev->private;
> +	return kzalloc(size, GFP_KERNEL);

That broke the driver :(

Please be more careful in your review.

thanks,

greg k-h

Re: [PATCH] staging: comedi: Compress return logic into one line.

[Varsha Rao]

[-- Attachment #1.1: Type: text/plain, Size: 243 bytes --]



>
> That broke the driver :( 
>
 

> Please be more careful in your review. 
>
   
    I had compiled it before submitting the patch and it did not cause any 
error or warning.
    Is there something else to be done?

    Thanks,
    Varsha

[-- Attachment #1.2: Type: text/html, Size: 582 bytes --]

Re: [Outreachy kernel] Re: [PATCH] staging: comedi: Compress return logic into one line.

[Greg KH]

On Sun, Mar 12, 2017 at 07:07:59AM -0700, Varsha Rao wrote:
> 
> 
>     That broke the driver :(
> 
> �
> 
>     Please be more careful in your review.
> 
> ��
> ��� I had compiled it before submitting the patch and it did not cause any
> error or warning.
> ��� Is there something else to be done?

Yes, you need to look at the logic that you are writing/changing to
determine that it is not incorrect.  This is valid C code, so no build
warning will happen, it's just the logic is totally wrong :)

You do see the problem, right?  If not, please take the time to read it
and figure out what is going on here, as it's important to know.

thanks,

greg k-h

Re: [Outreachy kernel] Re: [PATCH] staging: comedi: Compress return logic into one line.

[Greg KH]

On Sun, Mar 12, 2017 at 07:07:59AM -0700, Varsha Rao wrote:
> 
> 
>     That broke the driver :(
> 
> �
> 
>     Please be more careful in your review.
> 
> ��
> ��� I had compiled it before submitting the patch and it did not cause any
> error or warning.
> ��� Is there something else to be done?

Yes, you need to look at the logic that you are writing/changing to
determine that it is not incorrect.  This is valid C code, so no build
warning will happen, it's just the logic is totally wrong :)

You do see the problem, right?  If not, please take the time to read it
and figure out what is going on here, as it's important to know.

thanks,

greg k-h