
Hi Stephen,
2016-09-07 1:09 GMT+09:00 Stephen Warren swarren@wwwdotorg.org:
All child function calls are structured the same, so someone reading the code will always see the same structure irrespective of where in a function a child function is called. This gives uniformity. This also yields a few maintenance advantages below, and helps keep all code uniform even if any of the maintenance operations below have been applied to some functions and aren't needed in others.
Did you think I ran a semantic patch with Coccinelle and then sent it blindly?
No, this patch passed my eyes' check, at least.
Please notice this patch did not transform the following function in arch/arm/cpu/armv7/am33xx/clk_synthesizer.c
...
The patch description clearly stated that the patch was purely the result of applying the Coccinelle script. If there were exceptions or other edits, they should have been explicitly mentioned too.
Right. The git-log implied a semantic patch, but I did not mention that I sent the output of Coccinelle as is.
Actually, I cherry-picked reasonable hunks.
Coccinelle may sometimes do false positive jobs (or undesirable output like this case), so I think "Coccinelle + checking by eyes" is a good practice.
I dropped a semantic patch snippet from v3 git-log.
If we start to consider things that may happen or may not happen, we end up with adding redundancy all the time.
Are you positive or negative for the following hunk?
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c index f621f14..b27a6af 100644 --- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c @@ -788,11 +788,7 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr) if (ret) return ret;
ret = nand_register(devnum, mtd);
if (ret)
return ret;
return 0;
return nand_register(devnum, mtd);
}
I'd probably tend not to do that particular conversion, for consistency with the immediately preceding nand_scan_tail() case. Still, this one isn't such an obvious call so I wouldn't feel particularly strongly about it, especially as it isn't a driver I work on.
I think probe/init function can return a value of register function directly, from my best common sense.
This change will lose 2) in case fsl_elbc_chip_init() fails to do nand_register, though.
OK. I dropped those changes in v2.
(I still personally believe "return *_register();" is good coding style, though. This might be a matter of preference...)
In v3, I only fixed drivers/video/vidconsole-uclass.c because I thought it is simple enough. http://patchwork.ozlabs.org/patch/666560/
and it was acked by Anatolij.