* [PATCH] extcon: Fix return value in extcon_register_interest()
@ 2012-09-25 6:58 Sachin Kamat
2012-09-25 9:15 ` anish singh
2012-09-25 10:32 ` Chanwoo Choi
0 siblings, 2 replies; 6+ messages in thread
From: Sachin Kamat @ 2012-09-25 6:58 UTC (permalink / raw)
To: linux-kernel; +Cc: myungjoo.ham, cw00.choi, sachin.kamat, patches
Return the value obtained from extcon_find_cable_index()
instead of -ENODEV.
Fixes the following smatch info:
drivers/extcon/extcon-class.c:478 extcon_register_interest() info:
why not propagate 'obj->cable_index' from extcon_find_cable_index()
instead of -19?
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/extcon/extcon-class.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 946a318..e996800 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -475,7 +475,7 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
if (obj->cable_index < 0)
- return -ENODEV;
+ return obj->cable_index;
obj->user_nb = nb;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: Fix return value in extcon_register_interest()
2012-09-25 6:58 [PATCH] extcon: Fix return value in extcon_register_interest() Sachin Kamat
@ 2012-09-25 9:15 ` anish singh
2012-09-25 9:20 ` Sachin Kamat
2012-09-25 10:32 ` Chanwoo Choi
1 sibling, 1 reply; 6+ messages in thread
From: anish singh @ 2012-09-25 9:15 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-kernel, myungjoo.ham, cw00.choi, patches
On Tue, Sep 25, 2012 at 12:28 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Return the value obtained from extcon_find_cable_index()
> instead of -ENODEV.
>
> Fixes the following smatch info:
> drivers/extcon/extcon-class.c:478 extcon_register_interest() info:
> why not propagate 'obj->cable_index' from extcon_find_cable_index()
> instead of -19?
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> drivers/extcon/extcon-class.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index 946a318..e996800 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -475,7 +475,7 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
>
> obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
> if (obj->cable_index < 0)
> - return -ENODEV;
> + return obj->cable_index;
I think ENODEV is right here as the function will return negative
value only when
there is no such device for which the user is trying to register the interest.
Is there any problem with that?
>
> obj->user_nb = nb;
>
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: Fix return value in extcon_register_interest()
2012-09-25 9:15 ` anish singh
@ 2012-09-25 9:20 ` Sachin Kamat
2012-09-25 9:27 ` anish singh
0 siblings, 1 reply; 6+ messages in thread
From: Sachin Kamat @ 2012-09-25 9:20 UTC (permalink / raw)
To: anish singh; +Cc: linux-kernel, myungjoo.ham, cw00.choi, patches
On 25 September 2012 14:45, anish singh <anish198519851985@gmail.com> wrote:
> On Tue, Sep 25, 2012 at 12:28 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> Return the value obtained from extcon_find_cable_index()
>> instead of -ENODEV.
>>
>> Fixes the following smatch info:
>> drivers/extcon/extcon-class.c:478 extcon_register_interest() info:
>> why not propagate 'obj->cable_index' from extcon_find_cable_index()
>> instead of -19?
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>> drivers/extcon/extcon-class.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
>> index 946a318..e996800 100644
>> --- a/drivers/extcon/extcon-class.c
>> +++ b/drivers/extcon/extcon-class.c
>> @@ -475,7 +475,7 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
>>
>> obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
>> if (obj->cable_index < 0)
>> - return -ENODEV;
>> + return obj->cable_index;
> I think ENODEV is right here as the function will return negative
> value only when
> there is no such device for which the user is trying to register the interest.
> Is there any problem with that?
extcon_find_cable_index() returns -EINVAL when it fails.
Hence it is better to propagate the same error code instead of a different one.
>>
>> obj->user_nb = nb;
>>
>> --
>> 1.7.4.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: Fix return value in extcon_register_interest()
2012-09-25 9:20 ` Sachin Kamat
@ 2012-09-25 9:27 ` anish singh
0 siblings, 0 replies; 6+ messages in thread
From: anish singh @ 2012-09-25 9:27 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-kernel, myungjoo.ham, cw00.choi, patches
On Tue, Sep 25, 2012 at 2:50 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> On 25 September 2012 14:45, anish singh <anish198519851985@gmail.com> wrote:
>> On Tue, Sep 25, 2012 at 12:28 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>> Return the value obtained from extcon_find_cable_index()
>>> instead of -ENODEV.
>>>
>>> Fixes the following smatch info:
>>> drivers/extcon/extcon-class.c:478 extcon_register_interest() info:
>>> why not propagate 'obj->cable_index' from extcon_find_cable_index()
>>> instead of -19?
>>>
>>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>> ---
>>> drivers/extcon/extcon-class.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
>>> index 946a318..e996800 100644
>>> --- a/drivers/extcon/extcon-class.c
>>> +++ b/drivers/extcon/extcon-class.c
>>> @@ -475,7 +475,7 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
>>>
>>> obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
>>> if (obj->cable_index < 0)
>>> - return -ENODEV;
>>> + return obj->cable_index;
>> I think ENODEV is right here as the function will return negative
>> value only when
>> there is no such device for which the user is trying to register the interest.
>> Is there any problem with that?
>
> extcon_find_cable_index() returns -EINVAL when it fails.
> Hence it is better to propagate the same error code instead of a different one.
but returning wrong value wouldn't make sense IMHO.In this case
the user just want to register the interest for a particular device and what the
original author intends to return is this: there is no such device.I
think logically
it makes sense but I am not too sure about it.
>
>>>
>>> obj->user_nb = nb;
>>>
>>> --
>>> 1.7.4.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>
>
>
> --
> With warm regards,
> Sachin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: Fix return value in extcon_register_interest()
2012-09-25 6:58 [PATCH] extcon: Fix return value in extcon_register_interest() Sachin Kamat
2012-09-25 9:15 ` anish singh
@ 2012-09-25 10:32 ` Chanwoo Choi
2012-09-25 10:44 ` Sachin Kamat
1 sibling, 1 reply; 6+ messages in thread
From: Chanwoo Choi @ 2012-09-25 10:32 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-kernel, myungjoo.ham, patches
On 09/25/2012 03:58 PM, Sachin Kamat wrote:
> Return the value obtained from extcon_find_cable_index()
> instead of -ENODEV.
>
> Fixes the following smatch info:
> drivers/extcon/extcon-class.c:478 extcon_register_interest() info:
> why not propagate 'obj->cable_index' from extcon_find_cable_index()
> instead of -19?
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> drivers/extcon/extcon-class.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index 946a318..e996800 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -475,7 +475,7 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
>
> obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
> if (obj->cable_index < 0)
> - return -ENODEV;
> + return obj->cable_index;
>
> obj->user_nb = nb;
>
I agree.
But, if extcon_register_interest() return directly 'obj-cable_index' value
when extcon_find_cable_index() return -EINVAL, it would spoil readability
of extcon_register_interest() function. So, I think we can make it as
following
patch a little better.
---
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 936580b..078e6a5 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -468,7 +468,7 @@ int extcon_register_interest(struct
extcon_specific_cable_nb *obj,
obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
if (obj->cable_index < 0)
- return -ENODEV;
+ return -EINVAL;
obj->user_nb = nb;
Thanks,
Chanwoo Choi
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] extcon: Fix return value in extcon_register_interest()
2012-09-25 10:32 ` Chanwoo Choi
@ 2012-09-25 10:44 ` Sachin Kamat
0 siblings, 0 replies; 6+ messages in thread
From: Sachin Kamat @ 2012-09-25 10:44 UTC (permalink / raw)
To: Chanwoo Choi; +Cc: linux-kernel, myungjoo.ham, patches
On 25 September 2012 16:02, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> On 09/25/2012 03:58 PM, Sachin Kamat wrote:
>
>> Return the value obtained from extcon_find_cable_index()
>> instead of -ENODEV.
>>
>> Fixes the following smatch info:
>> drivers/extcon/extcon-class.c:478 extcon_register_interest() info:
>> why not propagate 'obj->cable_index' from extcon_find_cable_index()
>> instead of -19?
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>> drivers/extcon/extcon-class.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
>> index 946a318..e996800 100644
>> --- a/drivers/extcon/extcon-class.c
>> +++ b/drivers/extcon/extcon-class.c
>> @@ -475,7 +475,7 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
>>
>> obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
>> if (obj->cable_index < 0)
>> - return -ENODEV;
>> + return obj->cable_index;
>>
>> obj->user_nb = nb;
>>
>
>
> I agree.
> But, if extcon_register_interest() return directly 'obj-cable_index' value
> when extcon_find_cable_index() return -EINVAL, it would spoil readability
> of extcon_register_interest() function. So, I think we can make it as
> following
> patch a little better.
Ok. I will re-send this with this change.
>
> ---
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index 936580b..078e6a5 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -468,7 +468,7 @@ int extcon_register_interest(struct
> extcon_specific_cable_nb *obj,
>
> obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
> if (obj->cable_index < 0)
> - return -ENODEV;
> + return -EINVAL;
>
> obj->user_nb = nb;
>
>
>
> Thanks,
> Chanwoo Choi
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-25 10:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-25 6:58 [PATCH] extcon: Fix return value in extcon_register_interest() Sachin Kamat
2012-09-25 9:15 ` anish singh
2012-09-25 9:20 ` Sachin Kamat
2012-09-25 9:27 ` anish singh
2012-09-25 10:32 ` Chanwoo Choi
2012-09-25 10:44 ` Sachin Kamat
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.