Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1250807 > unrolled thread

Re: [PATCH v2] HID: hiddev: change hiddev_connect() to return bool

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2] HID: hiddev: change hiddev_connect() to return bool Luis de Bethencourt <luisbg@osg.samsung.com> - 2015-10-19 17:20 +0200
    Re: [PATCH v2] HID: hiddev: change hiddev_connect() to return bool Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-20 02:00 +0200

#1250807 — Re: [PATCH v2] HID: hiddev: change hiddev_connect() to return bool

On 13/10/15 02:49, Krzysztof Kozlowski wrote:
> 2015-10-09 22:00 GMT+09:00 Luis de Bethencourt <luisbg@osg.samsung.com>:
>> Since hid_connect() only cares about hiddev_connect() succeeding or
>> failing, there is no need for this function to return an int and it can
>> return a bool instead.
> 
> It can return bool but it would not be in line with kernel coding
> style. The hiddev_connect() I believe is an action, so "the function
> should return an error-code integer.".
> 
> Best regards,
> Krzysztof
>


Hi Krysztof,

The idea to switch the function to return bool was offered by Jiri Kosina,
as a result of my initial patch changing the return errno code to ENOMEM.

Considering the return isn't propagated by the only consumer of the function,
and your point about returning an integer being the kernel coding style. It
doesn't make sense to change this function.

Thanks for your review!
Luis
 
>>
>> Suggested-by: Jiri Kosina <jikos@kernel.org>
>> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
>> ---
>>
>> Hi,
>>
>> No idea why my local build did not complain about the obvious mistake
>> on the previous version of the patch.
>>
>> Sorry about that,
>> Luis
>>
>>  drivers/hid/hid-core.c      |  2 +-
>>  drivers/hid/usbhid/hiddev.c | 10 +++++-----
>>  include/linux/hid.h         |  2 +-
>>  include/linux/hiddev.h      |  2 +-
>>  4 files changed, 8 insertions(+), 8 deletions(-)

--
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/

[toc] | [next] | [standalone]

#1251163

On 20.10.2015 00:11, Luis de Bethencourt wrote:
> On 13/10/15 02:49, Krzysztof Kozlowski wrote:
>> 2015-10-09 22:00 GMT+09:00 Luis de Bethencourt <luisbg@osg.samsung.com>:
>>> Since hid_connect() only cares about hiddev_connect() succeeding or
>>> failing, there is no need for this function to return an int and it can
>>> return a bool instead.
>>
>> It can return bool but it would not be in line with kernel coding
>> style. The hiddev_connect() I believe is an action, so "the function
>> should return an error-code integer.".
>>
>> Best regards,
>> Krzysztof
>>
> 
> 
> Hi Krysztof,
> 
> The idea to switch the function to return bool was offered by Jiri Kosina,
> as a result of my initial patch changing the return errno code to ENOMEM.

I did not see the original comment from Jiri. Actually changing it to
ENOMEM makes more sense to me...

> 
> Considering the return isn't propagated by the only consumer of the function,
> and your point about returning an integer being the kernel coding style. It
> doesn't make sense to change this function.

Jiri is the maintainer here but for me sticking to coding convention
(return errno) makes it easier to read.

Best regards,
Krzysztof
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web