Date:	Tue, 17 Nov 2015 16:32:24 +0100 (CET)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	Boris Brezillon <boris.brezillon@...e-electrons.com>
cc:	Julia Lawall <julia.lawall@...6.fr>,
	Brian Norris <computersforpeace@...il.com>,
	David Woodhouse <dwmw2@...radead.org>,
	linux-mtd@...ts.infradead.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
	Hartley Sweeten <hsweeten@...ionengravers.com>,
	Ryan Mallon <rmallon@...il.com>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <kernel@...gutronix.de>,
	Imre Kaloz <kaloz@...nwrt.org>,
	Krzysztof Halasa <khalasa@...p.pl>,
	Tony Lindgren <tony@...mide.com>, linux-omap@...r.kernel.org,
	Alexander Clouter <alex@...riz.org.uk>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	Jason Cooper <jason@...edaemon.net>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Andrew Lunn <andrew@...n.ch>, Daniel Mack <daniel@...que.org>,
	Haojian Zhuang <haojian.zhuang@...il.com>,
	Robert Jarzmik <robert.jarzmik@...e.fr>,
	Marek Vasut <marek.vasut@...il.com>,
	Steven Miao <realmz6@...il.com>,
	adi-buildroot-devel@...ts.sourceforge.net,
	Mikael Starvik <starvik@...s.com>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	linux-cris-kernel@...s.com, Josh Wu <josh.wu@...el.com>,
	Wan ZongShun <mcuos.com@...il.com>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Maxim Levitsky <maximlevitsky@...il.com>,
	Kukjin Kim <kgene@...nel.org>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	linux-samsung-soc@...r.kernel.org,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Chen-Yu Tsai <wens@...e.org>, linux-sunxi@...glegroups.com,
	Stefan Agner <stefan@...er.ch>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	devel@...verdev.osuosl.org
Subject: Re: [PATCH 14/27] mtd: nand: use the mtd instance embedded in struct
 nand_chip

On Tue, 17 Nov 2015, Boris Brezillon wrote:

> Hi Julia,
>
> On Tue, 17 Nov 2015 10:05:03 +0100 (CET)
> Julia Lawall <julia.lawall@...6.fr> wrote:
>
> > > > (This isn't the worst one, but it just happens to be one of the first.)
> > > > There are many cases where the typical style would be to declare a new
> > > > variable at the top of the function, where you perform the
> > > > macro/function-call to convert from one abstraction to another. Like
> > > >
> > > > static int nfc_set_sram_bank(struct atmel_nand_host *host, unsigned int bank)
> > > > {
> > > > 	struct mtd_info *mtd = nand_to_mtd(&hot->nand_chip);
> > > > 	...
> > > >
> > > > and then use it later. Can that be done very easily?
> > > >
> > > > >  			return -EINVAL;
> > > > >  		nfc_writel(host->nfc->hsmc_regs, BANK, ATMEL_HSMC_NFC_BANK1);
> > > > >  	} else {
> > > >
> > > > ...
> > >
> > > Honestly, I don't know how to do that with a coccinelle script, and it
> > > will probably take me more time to find how to do it than addressing
> > > those problems manually.
> > >
> > > Julia, could you give us some hint?
> >
> > Probably something like the following would be easiest.  You can just run
> > it after your other transformations:
> >
> > @r exists@
> > identifier f;
> > expression e;
> > @@
> >
> > f(...) { <+...  nand_to_mtd(e) ...+> }
> >
> > @@
> > identifier r.f;
> > expression r.e;
> > @@
> >
> > f(...) {
> > + struct mtd_info *mtd = nand_to_mtd(e);
> > ...
> > }
>
> Thanks for the the suggestion...
>
> >
> > This won't work if there is more than one possible value of e.  If that is
> > likely, then I could come up with something more complex.  It also assumes
> > that you want to convert all such calls.  If you only want to convert calls
> > that occur in a particular context, eg a field reference, then you could
> > enhance the pattern inside the <+... ...+> in the first rule.
>
> ... unfortunately, as you've guessed, it's a bit more complicated.
> This mtd local variable is usually extracted from another local
> variable (struct nand_chip *chip), so we have to declare
>
> struct mtd_info *mtd = nand_to_mtd(e);
>
> after
>
> struct nand_chip *chip = expression;
>
> But this is not the only particular case. Sometime the chip variable is
> not assigned where it's declared (especially when it is dynamically
> allocated), and sometime it does not exist at all (we just extract it
> from a private struct: &priv->chip).
> The mtd variable can also be declared in sub-code blocks (loop or
> conditional statements).
> And, as you stated, we might want to keep direct calls to nand_to_mtd()
> if it's only called once in a function context.
>
> I'm pretty sure we could describe all those cases with specific context
> description, but I must admit that it takes less time for me to fix
> those specific cases manually than figuring out how to describe them
> correctly in a coccinelle script :).
>
> This being said, I'd be happy to see how you would handle all these
> different cases.

Maybe the following would be useful.  It won't handle all the cases, but
maybe it will take care of a good number of the nontrivial ones.

julia

@r@
local idexpression x;
identifier fld,y;
@@

nand_to_mtd(&x->fld)->y

@exists@
type T;
local idexpression r.x;
identifier xx,r.y,r.fld;
@@

T xx;
+ struct mtd_info *mtd = nand_to_mtd(&x->fld);
...
nand_to_mtd(&x@xx->fld)->y

@@
local idexpression r.x;
identifier r.fld;
@@

struct mtd_info *mtd = nand_to_mtd(&x->fld);
<...
- nand_to_mtd(&x->fld)
+ mtd
...>

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

Powered by blists - more mailing lists