On Tue, 2010-03-09 at 15:41 -0800, Greg KH wrote: > On Tue, Mar 09, 2010 at 03:32:06PM -0800, Joe Perches wrote: > > There was an article published recently: > > http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel > > that seems to have prompted several new contributors (welcome) > > to create style reformatting patches. > > > > The article recommends running checkpatch and fixing the various > > non-conforming style elements the output produces. > > > > A better solution might be to enhance checkpatch to rewrite the > > patch or a file with -f, but that's more than I'd like to do. > > I _really_ dislike automatic source conversions by a tool and do not > recommend doing that at all. Don't be silly. You take automatic conversions from tools like coccinelle all the time. What I found poorly written about the article was unfortunately you apparently recommend things like: if (error != -ENODEV) { foo(); bar(); } else { report_error(); goto exit; } instead of: if (error == -ENODEV) { report_error(); goto exit; } foo(); bar(); and the brace removal example was unfortunate because it used printk without KERN_<level> without explanation. I think there wasn't enough emphasis on compiling the new patched file with something like a .o or .lst comparison to the unmodified source to make sure it was OK to send to the list. > It's better that you look at the code yourself, and make the change that > looks correct, which is not always the same thing that an automated tool > would do. I disagree. It's better to make changes that _actually are_ correct. People also neglect to see and convert a lot of things. Tools can make coverage a bit better which is why a tool like checkpatch exists in the first place. > Becides, we have the Lindent script, which does almost exactly what you > are trying to do here, why reinvent the wheel? Lindent does a relatively poor job at conversions, and it's all or nothing. A tool that could be used to selectively perform various conversions would be better and could produce patches that are human verifiable. For instance, look at what happened with the old reiserfs changes. Lindent badly mucked up comments, did a poor job at columnarization of variable declarations, didn't understand c99 very well, was wretched at 80 columns, etc. I think the Lindent script should be deprecated and checkpatch enhanced myself. And until then, I think a script like cvt_kernel_style.pl has some use. cheers, Joe _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
