[PATCH v2] staging: media: davinci_vpfe: Rewrite return statement in vpfe_video.c

[PATCH v2] staging: media: davinci_vpfe: Rewrite return statement in vpfe_video.c

[Lisa Nguyen]

Rewrite the return statement in vpfe_video.c to eliminate the
use of a ternary operator. This will prevent the checkpatch.pl
script from generating a warning saying to remove () from
this particular return statement.

Signed-off-by: Lisa Nguyen <lisa@xenapiadmin.com>
---
Changes since v2:
- Aligned -ETIMEDOUT return statement with if condition

 drivers/staging/media/davinci_vpfe/vpfe_video.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 24d98a6..22e31d2 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -346,7 +346,10 @@ static int vpfe_pipeline_disable(struct vpfe_pipeline *pipe)
 	}
 	mutex_unlock(&mdev->graph_mutex);
 
-	return (ret == 0) ? ret : -ETIMEDOUT ;
+	if (ret == 0)
+		return ret;
+
+	return -ETIMEDOUT;
 }
 
 /*
-- 
1.7.9.5

Re: [PATCH v2] staging: media: davinci_vpfe: Rewrite return statement in vpfe_video.c

[Laurent Pinchart]

Hi Lisa,

Thank you for the patch.

On Tuesday 10 December 2013 08:05:42 Lisa Nguyen wrote:
> Rewrite the return statement in vpfe_video.c to eliminate the
> use of a ternary operator. This will prevent the checkpatch.pl
> script from generating a warning saying to remove () from
> this particular return statement.
> 
> Signed-off-by: Lisa Nguyen <lisa@xenapiadmin.com>
> ---
> Changes since v2:
> - Aligned -ETIMEDOUT return statement with if condition
> 
>  drivers/staging/media/davinci_vpfe/vpfe_video.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> b/drivers/staging/media/davinci_vpfe/vpfe_video.c index 24d98a6..22e31d2
> 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> @@ -346,7 +346,10 @@ static int vpfe_pipeline_disable(struct vpfe_pipeline
> *pipe) }
>  	mutex_unlock(&mdev->graph_mutex);
> 
> -	return (ret == 0) ? ret : -ETIMEDOUT ;
> +	if (ret == 0)
> +		return ret;
> +
> +	return -ETIMEDOUT;

I don't want to point the obvious, but what about just

	return ret ? -ETIMEDOUT : 0;

or, if this is just about fixing the checkpatch.pl warning,

	return ret == 0 ? ret : -ETIMEDOUT;

(I'd prefer the first)

>  }
> 
>  /*

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] staging: media: davinci_vpfe: Rewrite return statement in vpfe_video.c

[Lisa Nguyen]

Hi Laurent,

On Tue, Dec 10, 2013 at 8:50 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Lisa,
>
> Thank you for the patch.
>
> On Tuesday 10 December 2013 08:05:42 Lisa Nguyen wrote:
>> Rewrite the return statement in vpfe_video.c to eliminate the
>> use of a ternary operator. This will prevent the checkpatch.pl
>> script from generating a warning saying to remove () from
>> this particular return statement.
>>
>> Signed-off-by: Lisa Nguyen <lisa@xenapiadmin.com>
>> ---
>> Changes since v2:
>> - Aligned -ETIMEDOUT return statement with if condition
>>
>>  drivers/staging/media/davinci_vpfe/vpfe_video.c |    5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
>> b/drivers/staging/media/davinci_vpfe/vpfe_video.c index 24d98a6..22e31d2
>> 100644
>> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
>> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
>> @@ -346,7 +346,10 @@ static int vpfe_pipeline_disable(struct vpfe_pipeline
>> *pipe) }
>>       mutex_unlock(&mdev->graph_mutex);
>>
>> -     return (ret == 0) ? ret : -ETIMEDOUT ;
>> +     if (ret == 0)
>> +             return ret;
>> +
>> +     return -ETIMEDOUT;
>
> I don't want to point the obvious, but what about just
>
>         return ret ? -ETIMEDOUT : 0;
>
> or, if this is just about fixing the checkpatch.pl warning,
>
>         return ret == 0 ? ret : -ETIMEDOUT;
>
> (I'd prefer the first)

I understand your point :) I was making changes based on Prabhakar's
feedback he gave me a while back[1].

Should I wait until he says?

Lisa

[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg67833.html

Re: [PATCH v2] staging: media: davinci_vpfe: Rewrite return statement in vpfe_video.c

[Prabhakar Lad]

Hi Lisa,

On Tue, Dec 10, 2013 at 11:27 PM, Lisa Nguyen <lisa@xenapiadmin.com> wrote:
> Hi Laurent,
>
> On Tue, Dec 10, 2013 at 8:50 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>> Hi Lisa,
>>
>> Thank you for the patch.
>>
>> On Tuesday 10 December 2013 08:05:42 Lisa Nguyen wrote:
>>> Rewrite the return statement in vpfe_video.c to eliminate the
>>> use of a ternary operator. This will prevent the checkpatch.pl
>>> script from generating a warning saying to remove () from
>>> this particular return statement.
>>>
>>> Signed-off-by: Lisa Nguyen <lisa@xenapiadmin.com>
>>> ---
>>> Changes since v2:
>>> - Aligned -ETIMEDOUT return statement with if condition
>>>
>>>  drivers/staging/media/davinci_vpfe/vpfe_video.c |    5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
>>> b/drivers/staging/media/davinci_vpfe/vpfe_video.c index 24d98a6..22e31d2
>>> 100644
>>> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
>>> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
>>> @@ -346,7 +346,10 @@ static int vpfe_pipeline_disable(struct vpfe_pipeline
>>> *pipe) }
>>>       mutex_unlock(&mdev->graph_mutex);
>>>
>>> -     return (ret == 0) ? ret : -ETIMEDOUT ;
>>> +     if (ret == 0)
>>> +             return ret;
>>> +
>>> +     return -ETIMEDOUT;
>>
>> I don't want to point the obvious, but what about just
>>
>>         return ret ? -ETIMEDOUT : 0;
>>
>> or, if this is just about fixing the checkpatch.pl warning,
>>
>>         return ret == 0 ? ret : -ETIMEDOUT;
>>
>> (I'd prefer the first)
>
> I understand your point :) I was making changes based on Prabhakar's
> feedback he gave me a while back[1].
>
Please go ahead as per Laurent's suggestion.

Thanks,
--Prabhakar Lad