[PATCH] staging: comedi: Replace ternary operator with min macro

[PATCH] staging: comedi: Replace ternary operator with min macro

[Gargi Sharma]

Use macro min() to get the minimum of two values for
brevity and readability.

Found using Coccinelle:
@@ type T; T x; T y; @@
(
- x < y ? x : y
+ min(x,y)
|
- x > y ? x : y
+ max(x,y)
)

Signed-off-by: Gargi Sharma <gs051095@gmail.com>
---
 drivers/staging/comedi/drivers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index a5bf2cc..6a413c7 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -815,7 +815,7 @@ int comedi_load_firmware(struct comedi_device *dev,
 		release_firmware(fw);
 	}
 
-	return ret < 0 ? ret : 0;
+	return min(ret, 0);
 }
 EXPORT_SYMBOL_GPL(comedi_load_firmware);
 
-- 
2.7.4

Re: [Outreachy kernel] [PATCH] staging: comedi: Replace ternary operator with min macro

[Julia Lawall]

On Tue, 7 Mar 2017, Gargi Sharma wrote:

> Use macro min() to get the minimum of two values for
> brevity and readability.
>
> Found using Coccinelle:
> @@ type T; T x; T y; @@
> (
> - x < y ? x : y
> + min(x,y)
> |
> - x > y ? x : y
> + max(x,y)
> )
>
> Signed-off-by: Gargi Sharma <gs051095@gmail.com>

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

> ---
>  drivers/staging/comedi/drivers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> index a5bf2cc..6a413c7 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -815,7 +815,7 @@ int comedi_load_firmware(struct comedi_device *dev,
>  		release_firmware(fw);
>  	}
>
> -	return ret < 0 ? ret : 0;
> +	return min(ret, 0);
>  }
>  EXPORT_SYMBOL_GPL(comedi_load_firmware);
>
> --
> 2.7.4
>
> --
> 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/1488906954-25933-1-git-send-email-gs051095%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

RE: [PATCH] staging: comedi: Replace ternary operator with min macro

[Hartley Sweeten]

On Tuesday, March 07, 2017 10:16 AM, Gargi Sharma wrote:
>
> Use macro min() to get the minimum of two values for
> brevity and readability.
>
> Found using Coccinelle:
> @@ type T; T x; T y; @@
> (
> - x < y ? x : y
> + min(x,y)
> |
> - x > y ? x : y
> + max(x,y)
> )
> 
> Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> ---
>  drivers/staging/comedi/drivers.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> index a5bf2cc..6a413c7 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -815,7 +815,7 @@ int comedi_load_firmware(struct comedi_device *dev,
> 		release_firmware(fw);
>  	}
> 
> -	return ret < 0 ? ret : 0;
> +	return min(ret, 0);
>  }
>  EXPORT_SYMBOL_GPL(comedi_load_firmware);

Technically this patch is correct. But, logically it might be confusing.

This function either returns an error code (ret < 0) or success (0). The min() macro
makes it look like this function returns a minimum value.

Just my two cents...

Regards,
Hartley

Re: [Outreachy kernel] RE: [PATCH] staging: comedi: Replace ternary operator with min macro

[Julia Lawall]

On Tue, 7 Mar 2017, Hartley Sweeten wrote:

> On Tuesday, March 07, 2017 10:16 AM, Gargi Sharma wrote:
> >
> > Use macro min() to get the minimum of two values for
> > brevity and readability.
> >
> > Found using Coccinelle:
> > @@ type T; T x; T y; @@
> > (
> > - x < y ? x : y
> > + min(x,y)
> > |
> > - x > y ? x : y
> > + max(x,y)
> > )
> >
> > Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> > ---
> >  drivers/staging/comedi/drivers.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> > index a5bf2cc..6a413c7 100644
> > --- a/drivers/staging/comedi/drivers.c
> > +++ b/drivers/staging/comedi/drivers.c
> > @@ -815,7 +815,7 @@ int comedi_load_firmware(struct comedi_device *dev,
> > 		release_firmware(fw);
> >  	}
> >
> > -	return ret < 0 ? ret : 0;
> > +	return min(ret, 0);
> >  }
> >  EXPORT_SYMBOL_GPL(comedi_load_firmware);
>
> Technically this patch is correct. But, logically it might be confusing.
>
> This function either returns an error code (ret < 0) or success (0). The min() macro
> makes it look like this function returns a minimum value.

Agreed.  The ? is a bit ugly, but the min does not seem like the normal
way of returning an error code.

julia

>
> Just my two cents...
>
> Regards,
> Hartley
>
> --
> 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/SN1PR0101MB15652DA28CB022F0A3A8ED02D02F0%40SN1PR0101MB1565.prod.exchangelabs.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] RE: [PATCH] staging: comedi: Replace ternary operator with min macro

[Gargi Sharma]

On Tue, Mar 7, 2017 at 11:11 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Tue, 7 Mar 2017, Hartley Sweeten wrote:
>
>> On Tuesday, March 07, 2017 10:16 AM, Gargi Sharma wrote:
>> >
>> > Use macro min() to get the minimum of two values for
>> > brevity and readability.
>> >
>> > Found using Coccinelle:
>> > @@ type T; T x; T y; @@
>> > (
>> > - x < y ? x : y
>> > + min(x,y)
>> > |
>> > - x > y ? x : y
>> > + max(x,y)
>> > )
>> >
>> > Signed-off-by: Gargi Sharma <gs051095@gmail.com>
>> > ---
>> >  drivers/staging/comedi/drivers.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
>> > index a5bf2cc..6a413c7 100644
>> > --- a/drivers/staging/comedi/drivers.c
>> > +++ b/drivers/staging/comedi/drivers.c
>> > @@ -815,7 +815,7 @@ int comedi_load_firmware(struct comedi_device *dev,
>> >             release_firmware(fw);
>> >     }
>> >
>> > -   return ret < 0 ? ret : 0;
>> > +   return min(ret, 0);
>> >  }
>> >  EXPORT_SYMBOL_GPL(comedi_load_firmware);
>>
>> Technically this patch is correct. But, logically it might be confusing.
>>
>> This function either returns an error code (ret < 0) or success (0). The min() macro
>> makes it look like this function returns a minimum value.
>
> Agreed.  The ? is a bit ugly, but the min does not seem like the normal
> way of returning an error code.

Yeah, this makes sense. I'll redact this patch then. :)

thanks,
gargi
>
> julia
>
>>
>> Just my two cents...
>>
>> Regards,
>> Hartley
>>
>> --
>> 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/SN1PR0101MB15652DA28CB022F0A3A8ED02D02F0%40SN1PR0101MB1565.prod.exchangelabs.com.
>> For more options, visit https://groups.google.com/d/optout.
>>