2010-11-19 (ê), 07:57 -0500, Theodore Tso: > On Nov 19, 2010, at 5:13 AM, Namhyung Kim wrote: > > > * break long lines (using temp variables if needed) > > * merge short lines > > * put open brace on the same line > > * use C89-style comments > > * remove a space between function name and parenthesis > > * remove a space between '*' and pointer name > > * add a space after ',' > > * other random whitespace fixes > > > > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxx> > > What's the benefit of such massive cleanup patches, really? Does it > really enhance readability _that_ much? > > I believe in cleaning up code as I make substantive, useful change, but > code churn for code churn's sake has a number of downsides: > > *) It breaks other people's patches that might be pending (probably not > as much of an issue for ext3) > *) It makes it really easy to introduce > security holes in code (although it looks like --- I haven't checked > to make sure --- this shouldn't change the compiled code any so we can > at least audit this by applying the patch, and then checking to make > sure the .o hasn't changed. What really makes my skin crawl is a > massive change like that which doesn't have zero impact on the > compiled object code. If I get a patch like that, I reject it out of > hand for ext4.) > > Bottom line is I really don't think cleanup code helps a lot. It > wastes your time --- why not find some way of improving the kernel > that has more impact --- and it wastes the time of the responsible > maintainer (who has to go through the code with a fined-toothed comb > to make sure there's nothing bad hidden in a massive patch like this). > > Best regards, > > -- Ted > Hi Ted, I wrote this patch because checkpatch complains about the code when I tried to write another. Since I saw many codes in namei.c doesn't conform the kernel coding style so I decided to write this coding style patch first and others on top of it. But if you think it is totally useless, I'm fine with dropping it. BTW, I just checked that compiled code itself has no change on x86_64, but there was a change in .rodata section. Thanks. -- Regards, Namhyung Kim -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
