Hi Stephen, 2016-09-07 1:09 GMT+09:00 Stephen Warren <[email protected]>:
>>> 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. -- Best Regards Masahiro Yamada _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
