linux-kernel - Re: [PATCH] Input: introduce managed input devices (add devres support)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121029204023.GB13256@core.coreip.homeip.net>
Date:	Mon, 29 Oct 2012 13:40:23 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Henrik Rydberg <rydberg@...omail.se>
Cc:	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH] Input: introduce managed input devices (add devres
 support)

On Mon, Oct 29, 2012 at 09:02:26PM +0100, Henrik Rydberg wrote:
> > > > @@ -1766,8 +1830,14 @@ EXPORT_SYMBOL(input_allocate_device);
> > > >   */
> > > >  void input_free_device(struct input_dev *dev)
> > > >  {
> > > > -	if (dev)
> > > > +	if (dev) {
> > > > +		if (dev->devres_managed)
> > > > +			WARN_ON(devres_destroy(dev->dev.parent,
> > > > +						devm_input_device_release,
> > > > +						devm_input_device_match,
> > > > +						dev));
> > > >  		input_put_device(dev);
> > > 
> > > Device is put twice?
> > 
> > No, devres_destroy() does not actually run the release handler so we
> > need to call it explicitly.
> 
> Ok, I see it now - it merely uses the handler to qualify the matching object.
> 
> > > Why not add the resource to the input device instead? For one, it
> > > would make the order of unregister and release more apparent.
> > 
> > And what would that achieve? What would trigger unregistration?
> 
> As you say, it is a matter of view. We do not want to replay the whole
> "function with object argument or object with member function"
> debate. :-)
> 
> > > Right
> > > now, the code seems to rely on the reverse for-loop in the devres
> > > implementation.
> > 
> > That is the whole point of devres: it releases resources attached to
> > the parent device (either when ->probe() fails or after ->remove() is
> > called) in the opposed order of acquiring said resources. Think of it as
> > calling destructors in C++ code.
> 
> That's what I did, but I mapped register() to a member of the input
> resource, rather than to the parent device. If the parent device does
> not need to know how to unregister the input device, it makes sense to
> do so.
> 
> Either way, the code looks functional to me.

So is that "reviewed-by"?

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/