linux-kernel - Re: [PATCH 2/9] irqdomain: Introduce __irq_create

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]

Date:	Mon, 23 Sep 2013 22:29:36 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Wolfram Sang <wsa@...-dreams.de>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <rob.herring@...xeda.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 2/9] irqdomain: Introduce __irq_create_mapping()

On Mon, Sep 23, 2013 at 09:14:30PM +0200, Linus Walleij wrote:
> On Mon, Sep 16, 2013 at 10:31 AM, Thierry Reding
> <thierry.reding@...il.com> wrote:
> 
> > This is a version of irq_create_mapping() that propagates the precise
> > error code instead of returning 0 for all errors. It will be used in
> > subsequent patches to allow further propagation of error codes.
> >
> > To avoid code duplication, implement irq_create_mapping() as a wrapper
> > around the new __irq_create_mapping().
> >
> > Signed-off-by: Thierry Reding <treding@...dia.com>
> 
> Surprise! I don't like this.
> 
> I think it is better to first go over the call sites and make them
> all handle negative return numbers rather than pushing the
> obscure __interface.
> 
> I know from patch 0 that you think it's too much to change these
> 127 call sites but I don't think so, and I'm happy to merge one
> big patch changing all the 20 users in drivers/gpio.
> 
> Likewise with the 11 consumers in drivers/pinctrl.
> 
> It's just a a few archs+subsystems and it's just plain work.

Well, the problem is that the current patch changes the signature of the
function as well, therefore the call sites will have to be updated all
at once in a single patch to avoid build breakage. And that's excluding
any potential fallout from new callsites added between the creation of
the patch and its application.

Another alternative could be to change the signature in a way that does
not break compatibility. For instance I think it could work out if we
change this function to return int instead of unsigned int but keep the
same semantics to begin with (return 0 on failure). Then update all call
sites to handle potential negative errors and after that return negative
error codes. That still wouldn't catch any callers introduced between
the patch creation and application.

Thierry

Content of type "application/pgp-signature" skipped