All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis de Bethencourt <luisbg@osg.samsung.com>
To: Jiri Kosina <jikos@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-input@vger.kernel.org
Subject: Re: [PATCH] HID: hiddev: fix returned errno code in hiddev_connect()
Date: Sat, 03 Oct 2015 22:34:41 +0100	[thread overview]
Message-ID: <561049F1.5040507@osg.samsung.com> (raw)
In-Reply-To: <alpine.LNX.2.00.1509302138380.30132@pobox.suse.cz>

On 30/09/15 20:40, Jiri Kosina wrote:
> On Wed, 30 Sep 2015, Luis de Bethencourt wrote:
> 
>> The driver is using -1 instead of the -ENOMEM defined macro to specify
>> that a buffer allocation failed. Since the error number is propagated,
>> the caller will get a -EPERM which is the wrong error condition.
> 
> Generally I agree that the more specific errno, the better.
> 
> But I am not really sure where you are seeing the bug (mapping to -EPERM) 
> in this case? I think the only caller of hiddev_connect() should be 
> hid_connect(), and the only thing that guy cares about whether individual 
> callbacks succeed or fail, so that it sets hdev->clamed flags accordingly.
> 
> Could you please be more specific about the -EPERM mapping you are talking 
> about?
> 
> Thanks,
> 

I agree with you. The only caller of hiddev_connect() only checks if the
callback succeded. It checks if the return < 0.
What I meant is that -1 means -EPERM. [0]

This patch is purely about the correctness of using -ENOMEM. The word
"propagated" was not the best way to describe this problem. I could edit
the commit message if you would like.

Thanks for the review,
Luis

[0] http://lxr.free-electrons.com/source/include/uapi/asm-generic/errno-base.h#L15

  reply	other threads:[~2015-10-03 21:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-30  9:52 [PATCH] HID: hiddev: fix returned errno code in hiddev_connect() Luis de Bethencourt
2015-09-30  9:56 ` Luis de Bethencourt
2015-09-30 10:04   ` Sudip Mukherjee
2015-09-30 10:05     ` Luis de Bethencourt
2015-09-30 10:05       ` Luis de Bethencourt
2015-09-30 19:40 ` Jiri Kosina
2015-10-03 21:34   ` Luis de Bethencourt [this message]
2015-10-05 14:24     ` Jiri Kosina
2015-10-06  8:58       ` Luis de Bethencourt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=561049F1.5040507@osg.samsung.com \
    --to=luisbg@osg.samsung.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.