Blue Swirl <blauwir...@gmail.com> writes: > On Sat, Aug 21, 2010 at 9:54 AM, Markus Armbruster <arm...@redhat.com> wrote: >> Blue Swirl <blauwir...@gmail.com> writes: >> >>> On Fri, Aug 20, 2010 at 6:44 PM, Blue Swirl <blauwir...@gmail.com> wrote: >>>> On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster <arm...@redhat.com> >>>> wrote: >>>>> Anthony Liguori <anth...@codemonkey.ws> writes: >>>>>> To be perfectly honest, we have enough hard problems to solve in QEMU. >>>>>> We're spending a lot more time on coding style than we probably need >>>>>> to :-) >>>>> >>>>> In my not so humble opinion, that's because the current CODING_STYLE is >>>>> idiosyncratic, widely disliked (follows from idiosyncratic pretty much >>>>> inevitably), widely violated by existing code, and only haphazardly >>>>> enforced for new code. >>>> >>>> I think Coccinelle could help us here, it can check for some of the >>>> CODING_STYLE issues. We only need to include it to our build system >>>> and add git hooks if possible. It can also perform mechanical >>>> conversions (if desired). >>> >>> This Coccinelle script seems to do the job: >> [...] >>> There are some formatting issues though, I get changes like: >>> - for (i=0; i<5; i++) >>> + for(i = 0;i < 5;i++) { >>> >>> Reformatting the expressions with more spaces is nice, but removing >>> the spaces between 'for' and '(' and especially after ';' is not. >> >> Please make sure that patch submitters can easily check their patches >> with your tool. Depending on coccinelle isn't a problem for me, but it >> may well be for others. > > Coccinelle depends on OCaml and lots of other stuff, but just I > installed it with 'aptitude install coccinelle'. These days you can > also check Linux kernel sources with the included Coccinelle scripts.
Could be "fun" for developers using Windows. If they exist. > But depending on Coccinelle for the build wouldn't be nice. > >> Unless you mass-convert existing code to your style, tools working on >> source files won't cut it, because reports of the patch's style >> violations are prone to drown in a sea of reports of preexisting style >> violations. There's a reason why Linux's scrtips/checkpatch.pl works on >> patch files. > > Mass conversion would have the benefit that submitters, who use old > code as their reference, are more likely to use the correct style. > >> Even a working patch checking tool can only address the last issue >> (haphazard enforcement), not the other ones. You may not care. > > Which other ones? Quoting myself: [...] the current CODING_STYLE is idiosyncratic, widely disliked (follows from idiosyncratic pretty much inevitably), widely violated by existing code, and only haphazardly enforced for new code. >> I still think inventing yet another idiosyncratic coding style plus >> tools to enforce it is a waste of time. > > There are historical reasons for the style used in the current code > base. There are also reasons why CODING_STYLE was written like it > stands now. While wasting time for historical reasons is certainly better than wasting time for the heck of it, it's arguably worse than stopping the waste.