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@xxxxxxxxx> 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@xxxxxxxxxx> > > 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
Attachment:
pgpW8oU7QQfUE.pgp
Description: PGP signature
