Re: [PATCH 18/22] watchdog: mt7621_wdt: Use 'dev' instead of dereferencing it repeatedly

From: Guenter Roeck
Date: Wed Apr 10 2019 - 15:54:14 EST

On Wed, Apr 10, 2019 at 11:46:24AM -0700, Joe Perches wrote:
> On Wed, 2019-04-10 at 09:27 -0700, Guenter Roeck wrote:
> > Introduce local variable 'struct device *dev' and use it instead of
> > dereferencing it repeatedly.
> >
> > The conversion was done automatically with coccinelle using the
> > following semantic patches. The semantic patches and the scripts
> > used to generate this commit log are available at
> >
> Interesting collection. It would be useful to specify which
> particular script generated or enabled this patch.

It is pdev-addvar.cocci, rule 'new'. deref.cocci wasn't used for the
watchdog patches. The script to apply the various rules is watchdog/

Pointing to the actual scripts used is a good idea. I'll see if I can add
this for subsequent series. After all, the commit log is also auto-generated,
so this should be straightforward.

> Just scanning briefly, it might have been this one:
> But it looks like some manual bit might have been required too.

Not for this one. There were a couple of situations where I had to manually
split long lines to avoid checkpatch warnings, and I manually updated a few
of the commit logs, but not in this patch.

> And trivially:
> > diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c
> []
> > @@ -133,18 +133,19 @@ static struct watchdog_device mt7621_wdt_dev = {
> []
> > watchdog_init_timeout(&mt7621_wdt_dev, mt7621_wdt_dev.max_timeout,
> > - &pdev->dev);
> > + dev);
> This could be on one line.
Coccinelle isn't perfect. The rule doesn't modify the entire argument list,
only the last argument, so coccinelle missed that it could have merged the
two lines into one.

A checkpatch rule suggesting that multiple extension lines can be merged
might be useful to help finding such situations. Just a thought.