[Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

Add a few rules, based loosely on libvirt HACKING.

Blue Swirl (5):
  CODING_STYLE: add preprocessor rules
  CODING_STYLE: add C type rules
  CODING_STYLE: add memory management rules
  CODING_STYLE: add string management rules
  CODING_STYLE: add rules for printf-like functions

 CODING_STYLE |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 111 insertions(+), 0 deletions(-)

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[malc]

On Thu, 12 Aug 2010, Blue Swirl wrote:

> Add a few rules, based loosely on libvirt HACKING.
> 
> Blue Swirl (5):
>   CODING_STYLE: add preprocessor rules
>   CODING_STYLE: add C type rules
>   CODING_STYLE: add memory management rules
>   CODING_STYLE: add string management rules
>   CODING_STYLE: add rules for printf-like functions
> 
>  CODING_STYLE |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 111 insertions(+), 0 deletions(-)

While intentions of this are good, i believe this goes too far, i doubt
that the proposed additions are enforcable and have no doubts that they
will be widely ignored and at the same time provide more grounds for
whining. Furthermore the existing code doesn't follow them, going out on
a limb, it's more likely that one would look around the code he/she
modifies and base his/her modifications on the surrounding code than to
follow the style that conflicts with it.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Miguel Di Ciurcio Filho]

On Thu, Aug 12, 2010 at 3:56 PM, malc <av1474@comtv.ru> wrote:
>
> While intentions of this are good, i believe this goes too far, i doubt
> that the proposed additions are enforcable and have no doubts that they
> will be widely ignored and at the same time provide more grounds for
> whining. Furthermore the existing code doesn't follow them, going out on
> a limb, it's more likely that one would look around the code he/she
> modifies and base his/her modifications on the surrounding code than to
> follow the style that conflicts with it.

The existing code that I have touched don't follow the current coding
style guidance, much less all the new recommendations being suggested.

Although, I do believe that this situation needs to change. If we
agree in a coding style, I would volunteer to be a some kind of
observer to fix and alert people about coding styles mistakes.

Regards,

Miguel

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Fri, Aug 13, 2010 at 3:22 PM, Miguel Di Ciurcio Filho
<miguel.filho@gmail.com> wrote:
> On Thu, Aug 12, 2010 at 3:56 PM, malc <av1474@comtv.ru> wrote:
>>
>> While intentions of this are good, i believe this goes too far, i doubt
>> that the proposed additions are enforcable and have no doubts that they
>> will be widely ignored and at the same time provide more grounds for
>> whining. Furthermore the existing code doesn't follow them, going out on
>> a limb, it's more likely that one would look around the code he/she
>> modifies and base his/her modifications on the surrounding code than to
>> follow the style that conflicts with it.
>
> The existing code that I have touched don't follow the current coding
> style guidance, much less all the new recommendations being suggested.
>
> Although, I do believe that this situation needs to change. If we
> agree in a coding style, I would volunteer to be a some kind of
> observer to fix and alert people about coding styles mistakes.

I fully agree on the need of change and support your excellent idea.
There are other ways to solve the problem, but I believe we need more
order than more chaos. Perhaps we the QEMU developers should appoint
you the Guardian of the CODING_STYLE, and add a rule that no patch
shall be committed without your CS-Acked-by line?

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/13/10 20:02, Blue Swirl wrote:
> On Fri, Aug 13, 2010 at 3:22 PM, Miguel Di Ciurcio Filho
> <miguel.filho@gmail.com> wrote:
>> The existing code that I have touched don't follow the current coding
>> style guidance, much less all the new recommendations being suggested.
>>
>> Although, I do believe that this situation needs to change. If we
>> agree in a coding style, I would volunteer to be a some kind of
>> observer to fix and alert people about coding styles mistakes.
> 
> I fully agree on the need of change and support your excellent idea.
> There are other ways to solve the problem, but I believe we need more
> order than more chaos. Perhaps we the QEMU developers should appoint
> you the Guardian of the CODING_STYLE, and add a rule that no patch
> shall be committed without your CS-Acked-by line?

I don't think this would ever work, it is begging for trouble relying on
one person to review all patches for this.

While I agree coding style is good since it enforces consistency, there
are plenty problems with the old rules. For example the rule to demand
braces around single line in an if statement. It results in more empty
lines on the screeen, ie lost screen real estate making it harder for
someone reading the code to get a good overview.

If we are going to mod the QEMU coding style rules, I strongly recommend
we look at the Linux kernel rules, while keeping the 4 space
indentation, rather than trying to adopt things from libvirt.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Anthony Liguori]

On 08/17/2010 03:04 AM, Jes Sorensen wrote:
> On 08/13/10 20:02, Blue Swirl wrote:
>    
>> On Fri, Aug 13, 2010 at 3:22 PM, Miguel Di Ciurcio Filho
>> <miguel.filho@gmail.com>  wrote:
>>      
>>> The existing code that I have touched don't follow the current coding
>>> style guidance, much less all the new recommendations being suggested.
>>>
>>> Although, I do believe that this situation needs to change. If we
>>> agree in a coding style, I would volunteer to be a some kind of
>>> observer to fix and alert people about coding styles mistakes.
>>>        
>> I fully agree on the need of change and support your excellent idea.
>> There are other ways to solve the problem, but I believe we need more
>> order than more chaos. Perhaps we the QEMU developers should appoint
>> you the Guardian of the CODING_STYLE, and add a rule that no patch
>> shall be committed without your CS-Acked-by line?
>>      
> I don't think this would ever work, it is begging for trouble relying on
> one person to review all patches for this.
>
> While I agree coding style is good since it enforces consistency, there
> are plenty problems with the old rules

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 :-)

Regards,

Anthony Liguori

> . For example the rule to demand
> braces around single line in an if statement. It results in more empty
> lines on the screeen, ie lost screen real estate making it harder for
> someone reading the code to get a good overview.
>
> If we are going to mod the QEMU coding style rules, I strongly recommend
> we look at the Linux kernel rules, while keeping the 4 space
> indentation, rather than trying to adopt things from libvirt.
>
> Cheers,
> Jes
>
>    

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/17/10 15:21, Anthony Liguori wrote:
> On 08/17/2010 03:04 AM, Jes Sorensen wrote:
>> On 08/13/10 20:02, Blue Swirl wrote:      
>>> I fully agree on the need of change and support your excellent idea.
>>> There are other ways to solve the problem, but I believe we need more
>>> order than more chaos. Perhaps we the QEMU developers should appoint
>>> you the Guardian of the CODING_STYLE, and add a rule that no patch
>>> shall be committed without your CS-Acked-by line?
>>>      
>> I don't think this would ever work, it is begging for trouble relying on
>> one person to review all patches for this.
>>
>> While I agree coding style is good since it enforces consistency, there
>> are plenty problems with the old rules
> 
> 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 :-)

I think we are in full agreement here, I am really just worried that we
add additional procedures here that will slow down the real development.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Tue, Aug 17, 2010 at 1:55 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 08/17/10 15:21, Anthony Liguori wrote:
>> On 08/17/2010 03:04 AM, Jes Sorensen wrote:
>>> On 08/13/10 20:02, Blue Swirl wrote:
>>>> I fully agree on the need of change and support your excellent idea.
>>>> There are other ways to solve the problem, but I believe we need more
>>>> order than more chaos. Perhaps we the QEMU developers should appoint
>>>> you the Guardian of the CODING_STYLE, and add a rule that no patch
>>>> shall be committed without your CS-Acked-by line?
>>>>
>>> I don't think this would ever work, it is begging for trouble relying on
>>> one person to review all patches for this.
>>>
>>> While I agree coding style is good since it enforces consistency, there
>>> are plenty problems with the old rules
>>
>> 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 :-)
>
> I think we are in full agreement here, I am really just worried that we
> add additional procedures here that will slow down the real development.

Either the braces rule in CODING_STYLE is downgraded to
recommendation, or some kind of new order is introduced. Current
situation is unfair.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/17/10 20:56, Blue Swirl wrote:
> On Tue, Aug 17, 2010 at 1:55 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>> I think we are in full agreement here, I am really just worried that we
>> add additional procedures here that will slow down the real development.
> 
> Either the braces rule in CODING_STYLE is downgraded to
> recommendation, or some kind of new order is introduced. Current
> situation is unfair.

Just to be sure I follow, are you suggesting we relax all of the bracing
rule, or just the part about braces around single line statements? I'd
be happy to write up a patch for the latter.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Thu, Aug 19, 2010 at 1:32 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 08/17/10 20:56, Blue Swirl wrote:
>> On Tue, Aug 17, 2010 at 1:55 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>>> I think we are in full agreement here, I am really just worried that we
>>> add additional procedures here that will slow down the real development.
>>
>> Either the braces rule in CODING_STYLE is downgraded to
>> recommendation, or some kind of new order is introduced. Current
>> situation is unfair.
>
> Just to be sure I follow, are you suggesting we relax all of the bracing
> rule, or just the part about braces around single line statements? I'd
> be happy to write up a patch for the latter.

I'd rather not relax the rules but find a solution so that the rules work.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/19/2010 09:29 PM, Blue Swirl wrote:
>
>> Just to be sure I follow, are you suggesting we relax all of the bracing
>> rule, or just the part about braces around single line statements? I'd
>> be happy to write up a patch for the latter.
> I'd rather not relax the rules but find a solution so that the rules work.
>

I happen to like the single line braces rule.  That is, I don't like how 
the code looks (I dislike punctuation generally), but I like the 
consistency and I like how patches that add or remove a line are easy to 
read.

My preference would be: new code has to adhere to the new style.

Perhaps we can have a bot subscribed to the list issue auto-reviews if 
things are incorrect.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/22/10 22:15, Avi Kivity wrote:
>  On 08/19/2010 09:29 PM, Blue Swirl wrote:
>>
>>> Just to be sure I follow, are you suggesting we relax all of the bracing
>>> rule, or just the part about braces around single line statements? I'd
>>> be happy to write up a patch for the latter.
>> I'd rather not relax the rules but find a solution so that the rules
>> work.
> 
> I happen to like the single line braces rule.  That is, I don't like how
> the code looks (I dislike punctuation generally), but I like the
> consistency and I like how patches that add or remove a line are easy to
> read.
> 
> My preference would be: new code has to adhere to the new style.

Agreed, once we pick something, make the new stuff stick to that. The
main issue is when you make mods to a file that is using a different
style. It's not ideal that you add one line of the new style and the
rest is the old way.

> Perhaps we can have a bot subscribed to the list issue auto-reviews if
> things are incorrect.

That might work.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/22/2010 11:20 PM, Jes Sorensen wrote:
>
>> I happen to like the single line braces rule.  That is, I don't like how
>> the code looks (I dislike punctuation generally), but I like the
>> consistency and I like how patches that add or remove a line are easy to
>> read.
>>
>> My preference would be: new code has to adhere to the new style.
> Agreed, once we pick something, make the new stuff stick to that. The
> main issue is when you make mods to a file that is using a different
> style. It's not ideal that you add one line of the new style and the
> rest is the old way.

I happen not to care too much but that's a personal thing I guess.

>> Perhaps we can have a bot subscribed to the list issue auto-reviews if
>> things are incorrect.
> That might work.

Anyone for a Google Weekend of Code?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 08/17/2010 03:04 AM, Jes Sorensen wrote:
>> On 08/13/10 20:02, Blue Swirl wrote:
>>    
>>> On Fri, Aug 13, 2010 at 3:22 PM, Miguel Di Ciurcio Filho
>>> <miguel.filho@gmail.com>  wrote:
>>>      
>>>> The existing code that I have touched don't follow the current coding
>>>> style guidance, much less all the new recommendations being suggested.
>>>>
>>>> Although, I do believe that this situation needs to change. If we
>>>> agree in a coding style, I would volunteer to be a some kind of
>>>> observer to fix and alert people about coding styles mistakes.
>>>>        
>>> I fully agree on the need of change and support your excellent idea.
>>> There are other ways to solve the problem, but I believe we need more
>>> order than more chaos. Perhaps we the QEMU developers should appoint
>>> you the Guardian of the CODING_STYLE, and add a rule that no patch
>>> shall be committed without your CS-Acked-by line?
>>>      
>> I don't think this would ever work, it is begging for trouble relying on
>> one person to review all patches for this.
>>
>> While I agree coding style is good since it enforces consistency, there
>> are plenty problems with the old rules
>
> 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'd support switching to Linux kernel style.  Then we can point people
complaining about it to Linux (good luck getting it changed there), use
Linux's tools to check for compliance (beats building our own), and move
on to more productive issues.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Anthony Liguori <anthony@codemonkey.ws> writes:
>
>> On 08/17/2010 03:04 AM, Jes Sorensen wrote:
>>> On 08/13/10 20:02, Blue Swirl wrote:
>>>
>>>> On Fri, Aug 13, 2010 at 3:22 PM, Miguel Di Ciurcio Filho
>>>> <miguel.filho@gmail.com>  wrote:
>>>>
>>>>> The existing code that I have touched don't follow the current coding
>>>>> style guidance, much less all the new recommendations being suggested.
>>>>>
>>>>> Although, I do believe that this situation needs to change. If we
>>>>> agree in a coding style, I would volunteer to be a some kind of
>>>>> observer to fix and alert people about coding styles mistakes.
>>>>>
>>>> I fully agree on the need of change and support your excellent idea.
>>>> There are other ways to solve the problem, but I believe we need more
>>>> order than more chaos. Perhaps we the QEMU developers should appoint
>>>> you the Guardian of the CODING_STYLE, and add a rule that no patch
>>>> shall be committed without your CS-Acked-by line?
>>>>
>>> I don't think this would ever work, it is begging for trouble relying on
>>> one person to review all patches for this.
>>>
>>> While I agree coding style is good since it enforces consistency, there
>>> are plenty problems with the old rules
>>
>> 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).

> I'd support switching to Linux kernel style.  Then we can point people
> complaining about it to Linux (good luck getting it changed there), use
> Linux's tools to check for compliance (beats building our own), and move
> on to more productive issues.

Not again:
http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Fri, Aug 20, 2010 at 6:44 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> Anthony Liguori <anthony@codemonkey.ws> writes:
>>
>>> On 08/17/2010 03:04 AM, Jes Sorensen wrote:
>>>> On 08/13/10 20:02, Blue Swirl wrote:
>>>>
>>>>> On Fri, Aug 13, 2010 at 3:22 PM, Miguel Di Ciurcio Filho
>>>>> <miguel.filho@gmail.com>  wrote:
>>>>>
>>>>>> The existing code that I have touched don't follow the current coding
>>>>>> style guidance, much less all the new recommendations being suggested.
>>>>>>
>>>>>> Although, I do believe that this situation needs to change. If we
>>>>>> agree in a coding style, I would volunteer to be a some kind of
>>>>>> observer to fix and alert people about coding styles mistakes.
>>>>>>
>>>>> I fully agree on the need of change and support your excellent idea.
>>>>> There are other ways to solve the problem, but I believe we need more
>>>>> order than more chaos. Perhaps we the QEMU developers should appoint
>>>>> you the Guardian of the CODING_STYLE, and add a rule that no patch
>>>>> shall be committed without your CS-Acked-by line?
>>>>>
>>>> I don't think this would ever work, it is begging for trouble relying on
>>>> one person to review all patches for this.
>>>>
>>>> While I agree coding style is good since it enforces consistency, there
>>>> are plenty problems with the old rules
>>>
>>> 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:
@curly_if@
position p;
@@

if@p (...) { ... }

@simple_if@
position p != curly_if.p;
statement S;
expression E;
@@

- if@p (E) S
+ if (E) {
+    S
+ }

@curly_for@
position q;
@@

for@q (...;...;...) { ... }

@simple_for@
position q != curly_for.q;
statement S;
expression E1, E2, E3;
@@

- for@q (E1; E2; E3) S
+ for (E1; E2; E3) {
+    S
+ }

@curly_while@
position r;
@@

while@r (...) { ... }

@simple_while@
position r != curly_while.r;
statement S;
expression E;
@@

- while@r (E) S
+ while (E) {
+    S
+ }

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.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Markus Armbruster]

Blue Swirl <blauwirbel@gmail.com> writes:

> On Fri, Aug 20, 2010 at 6:44 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster <armbru@redhat.com> wrote:
>>> Anthony Liguori <anthony@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.

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.

Even a working patch checking tool can only address the last issue
(haphazard enforcement), not the other ones.  You may not care.

I still think inventing yet another idiosyncratic coding style plus
tools to enforce it is a waste of time.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sat, Aug 21, 2010 at 9:54 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> On Fri, Aug 20, 2010 at 6:44 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>>> On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster <armbru@redhat.com> wrote:
>>>> Anthony Liguori <anthony@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.

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?

> 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.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Markus Armbruster]

Blue Swirl <blauwirbel@gmail.com> writes:

> On Sat, Aug 21, 2010 at 9:54 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> Blue Swirl <blauwirbel@gmail.com> writes:
>>
>>> On Fri, Aug 20, 2010 at 6:44 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>> On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster <armbru@redhat.com> wrote:
>>>>> Anthony Liguori <anthony@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.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sat, Aug 21, 2010 at 12:24 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> On Sat, Aug 21, 2010 at 9:54 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>> Blue Swirl <blauwirbel@gmail.com> writes:
>>>
>>>> On Fri, Aug 20, 2010 at 6:44 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>>> On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster <armbru@redhat.com> wrote:
>>>>>> Anthony Liguori <anthony@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.

At least OCaml site offers binary download for Windows. I didn't
compile Coccinelle myself, so I don't know how much that helps.

>> 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,

Personal preference. I liked Fabrice's style but I also like current
style. I would probably like Linux style except for the LISPisms. I
don't like GNU or Java style.

> widely disliked (follows from idiosyncratic pretty much
>    inevitably),

How widely? How do you know that?

> widely violated by existing code,

The mechanical conversion would address that. We could of course
convert to some other style, or declare that we don't care about the
style after all. Or claim that we care, don't fix old code and whine
randomly at some submitters.

> and only haphazardly
>    enforced for new code.

I'd like to fix that.

>>> 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.

But how would you do that? Drop the CODING_STYLE (and accept
anything)? Switch to a new CODING_STYLE that is widely appreciated and
so all bikeshedding will cease? Enforce current style?

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/21/10 16:03, Blue Swirl wrote:
> On Sat, Aug 21, 2010 at 12:24 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> Could be "fun" for developers using Windows.  If they exist.
> 
> At least OCaml site offers binary download for Windows. I didn't
> compile Coccinelle myself, so I don't know how much that helps.

I know nothing about Coccinelle, but I did find that yum knew where to
get it. However, that said, I think we should try to avoid depending on
exotic tools that may not exist on OSes which may be used by developers.
What about OSX?

>>>> 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,
> 
> Personal preference. I liked Fabrice's style but I also like current
> style. I would probably like Linux style except for the LISPisms. I
> don't like GNU or Java style.

My favorite quote from the Linux kernel coding style:
"First off, I'd suggest printing out a copy of the GNU coding standards,
and NOT read it.  Burn them, it's a great symbolic gesture." :)

>> 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.
> 
> But how would you do that? Drop the CODING_STYLE (and accept
> anything)? Switch to a new CODING_STYLE that is widely appreciated and
> so all bikeshedding will cease? Enforce current style?

I would suggest we either clean up the existing rule, or switch to the
Linux kernel style, with the explicit exemption that existing code can
keep the 4-char indentation, unless the whole file is converted. I'd
like to avoid a total reformatting of the codebase, but we could look at
it on a file by file base if it becomes relevant.

Regards,
Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[malc]

On Sun, 22 Aug 2010, Jes Sorensen wrote:

> On 08/21/10 16:03, Blue Swirl wrote:
> > On Sat, Aug 21, 2010 at 12:24 PM, Markus Armbruster <armbru@redhat.com> wrote:
> >> Could be "fun" for developers using Windows.  If they exist.
> > 
> > At least OCaml site offers binary download for Windows. I didn't
> > compile Coccinelle myself, so I don't know how much that helps.
> 
> I know nothing about Coccinelle, but I did find that yum knew where to
> get it. However, that said, I think we should try to avoid depending on
> exotic tools that may not exist on OSes which may be used by developers.
> What about OSX?
> 
> >>>> 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,
> > 
> > Personal preference. I liked Fabrice's style but I also like current
> > style. I would probably like Linux style except for the LISPisms. I
> > don't like GNU or Java style.
> 
> My favorite quote from the Linux kernel coding style:
> "First off, I'd suggest printing out a copy of the GNU coding standards,
> and NOT read it.  Burn them, it's a great symbolic gesture." :)
> 
> >> 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.
> > 
> > But how would you do that? Drop the CODING_STYLE (and accept
> > anything)? Switch to a new CODING_STYLE that is widely appreciated and
> > so all bikeshedding will cease? Enforce current style?

Let's burn GCC, binutils and the rest of the stuff written in this style
too, the fact that Linus uses inferior editor is not good enough reson
to follow his style nor advice. That said, second sentence of the opening
paragraph of Linux's coding style document does resonate with me.

> 
> I would suggest we either clean up the existing rule, or switch to the
> Linux kernel style, with the explicit exemption that existing code can
> keep the 4-char indentation, unless the whole file is converted. I'd
> like to avoid a total reformatting of the codebase, but we could look at
> it on a file by file base if it becomes relevant.
> 
> Regards,
> Jes
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sun, Aug 22, 2010 at 5:00 PM, malc <av1474@comtv.ru> wrote:
> On Sun, 22 Aug 2010, Jes Sorensen wrote:
>
>> On 08/21/10 16:03, Blue Swirl wrote:
>> > On Sat, Aug 21, 2010 at 12:24 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> >> Could be "fun" for developers using Windows.  If they exist.
>> >
>> > At least OCaml site offers binary download for Windows. I didn't
>> > compile Coccinelle myself, so I don't know how much that helps.
>>
>> I know nothing about Coccinelle, but I did find that yum knew where to
>> get it. However, that said, I think we should try to avoid depending on
>> exotic tools that may not exist on OSes which may be used by developers.
>> What about OSX?
>>
>> >>>> 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,
>> >
>> > Personal preference. I liked Fabrice's style but I also like current
>> > style. I would probably like Linux style except for the LISPisms. I
>> > don't like GNU or Java style.
>>
>> My favorite quote from the Linux kernel coding style:
>> "First off, I'd suggest printing out a copy of the GNU coding standards,
>> and NOT read it.  Burn them, it's a great symbolic gesture." :)
>>
>> >> 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.
>> >
>> > But how would you do that? Drop the CODING_STYLE (and accept
>> > anything)? Switch to a new CODING_STYLE that is widely appreciated and
>> > so all bikeshedding will cease? Enforce current style?
>
> Let's burn GCC, binutils and the rest of the stuff written in this style
> too, the fact that Linus uses inferior editor is not good enough reson
> to follow his style nor advice. That said, second sentence of the opening
> paragraph of Linux's coding style document does resonate with me.

Even if GCC etc. were written in LISP and then preprocessed into C
(which seems to be the intention of the developers), I'd still use
them. Especially since there are very few alternatives.

OCaml looks very uninteresting or even painful to me. I still have no
problem using Coccinelle, as long as I don't have to fix bugs by
looking into its sources.

What's the ultimate editor? I'd love to drop Emacs, which is annoying
but does its job better than the others that I've tried.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2761 bytes --]

On Sun, 22 Aug 2010, Blue Swirl wrote:

> On Sun, Aug 22, 2010 at 5:00 PM, malc <av1474@comtv.ru> wrote:
> > On Sun, 22 Aug 2010, Jes Sorensen wrote:
> >
> >> On 08/21/10 16:03, Blue Swirl wrote:
> >> > On Sat, Aug 21, 2010 at 12:24 PM, Markus Armbruster <armbru@redhat.com> wrote:
> >> >> Could be "fun" for developers using Windows.  If they exist.
> >> >
> >> > At least OCaml site offers binary download for Windows. I didn't
> >> > compile Coccinelle myself, so I don't know how much that helps.
> >>
> >> I know nothing about Coccinelle, but I did find that yum knew where to
> >> get it. However, that said, I think we should try to avoid depending on
> >> exotic tools that may not exist on OSes which may be used by developers.
> >> What about OSX?
> >>
> >> >>>> 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,
> >> >
> >> > Personal preference. I liked Fabrice's style but I also like current
> >> > style. I would probably like Linux style except for the LISPisms. I
> >> > don't like GNU or Java style.
> >>
> >> My favorite quote from the Linux kernel coding style:
> >> "First off, I'd suggest printing out a copy of the GNU coding standards,
> >> and NOT read it.  Burn them, it's a great symbolic gesture." :)
> >>
> >> >> 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.
> >> >
> >> > But how would you do that? Drop the CODING_STYLE (and accept
> >> > anything)? Switch to a new CODING_STYLE that is widely appreciated and
> >> > so all bikeshedding will cease? Enforce current style?
> >
> > Let's burn GCC, binutils and the rest of the stuff written in this style
> > too, the fact that Linus uses inferior editor is not good enough reson
> > to follow his style nor advice. That said, second sentence of the opening
> > paragraph of Linux's coding style document does resonate with me.
> 
> Even if GCC etc. were written in LISP and then preprocessed into C
> (which seems to be the intention of the developers), I'd still use
> them. Especially since there are very few alternatives.
> 
> OCaml looks very uninteresting or even painful to me. I still have no
> problem using Coccinelle, as long as I don't have to fix bugs by
> looking into its sources.
> 
> What's the ultimate editor? I'd love to drop Emacs, which is annoying
> but does its job better than the others that I've tried.
> 

ed

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/22/10 20:35, malc wrote:
> On Sun, 22 Aug 2010, Blue Swirl wrote:
>> What's the ultimate editor? I'd love to drop Emacs, which is annoying
>> but does its job better than the others that I've tried.
>>
> 
> ed

ed is for sissies, real developers use cat, echo, and sed.....

Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[john cooper]

Jes Sorensen wrote:
> On 08/22/10 20:35, malc wrote:
>> On Sun, 22 Aug 2010, Blue Swirl wrote:
>>> What's the ultimate editor? I'd love to drop Emacs, which is annoying
>>> but does its job better than the others that I've tried.
>>>       
>> ed
>
> ed is for sissies, real developers use cat, echo, and sed.....
I prefer teco -- its smaller footprint consumes far less
core.  I find this also improves user interactivity on a
heavily loaded system when swapping to drum.

-john

$$

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sun, Aug 22, 2010 at 4:49 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 08/21/10 16:03, Blue Swirl wrote:
>> On Sat, Aug 21, 2010 at 12:24 PM, Markus Armbruster <armbru@redhat.com> wrote:
>>> Could be "fun" for developers using Windows.  If they exist.
>>
>> At least OCaml site offers binary download for Windows. I didn't
>> compile Coccinelle myself, so I don't know how much that helps.
>
> I know nothing about Coccinelle, but I did find that yum knew where to
> get it. However, that said, I think we should try to avoid depending on
> exotic tools that may not exist on OSes which may be used by developers.
> What about OSX?

Same thing, binary for OCaml exists. There's none for *BSD or *Solaris, though.

>>>>> 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,
>>
>> Personal preference. I liked Fabrice's style but I also like current
>> style. I would probably like Linux style except for the LISPisms. I
>> don't like GNU or Java style.
>
> My favorite quote from the Linux kernel coding style:
> "First off, I'd suggest printing out a copy of the GNU coding standards,
> and NOT read it.  Burn them, it's a great symbolic gesture." :)
>
>>> 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.
>>
>> But how would you do that? Drop the CODING_STYLE (and accept
>> anything)? Switch to a new CODING_STYLE that is widely appreciated and
>> so all bikeshedding will cease? Enforce current style?
>
> I would suggest we either clean up the existing rule, or switch to the
> Linux kernel style, with the explicit exemption that existing code can
> keep the 4-char indentation, unless the whole file is converted. I'd
> like to avoid a total reformatting of the codebase, but we could look at
> it on a file by file base if it becomes relevant.

Sounds reasonable.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2382 bytes --]

On Sun, 22 Aug 2010, Blue Swirl wrote:

> On Sun, Aug 22, 2010 at 4:49 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> > On 08/21/10 16:03, Blue Swirl wrote:
> >> On Sat, Aug 21, 2010 at 12:24 PM, Markus Armbruster <armbru@redhat.com> wrote:
> >>> Could be "fun" for developers using Windows.  If they exist.
> >>
> >> At least OCaml site offers binary download for Windows. I didn't
> >> compile Coccinelle myself, so I don't know how much that helps.
> >
> > I know nothing about Coccinelle, but I did find that yum knew where to
> > get it. However, that said, I think we should try to avoid depending on
> > exotic tools that may not exist on OSes which may be used by developers.
> > What about OSX?
> 
> Same thing, binary for OCaml exists. There's none for *BSD or *Solaris, 
> though.

FWIW OCaml is bootstrapable using only C.
 
> >>>>> 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,
> >>
> >> Personal preference. I liked Fabrice's style but I also like current
> >> style. I would probably like Linux style except for the LISPisms. I
> >> don't like GNU or Java style.
> >
> > My favorite quote from the Linux kernel coding style:
> > "First off, I'd suggest printing out a copy of the GNU coding standards,
> > and NOT read it.  Burn them, it's a great symbolic gesture." :)
> >
> >>> 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.
> >>
> >> But how would you do that? Drop the CODING_STYLE (and accept
> >> anything)? Switch to a new CODING_STYLE that is widely appreciated and
> >> so all bikeshedding will cease? Enforce current style?
> >
> > I would suggest we either clean up the existing rule, or switch to the
> > Linux kernel style, with the explicit exemption that existing code can
> > keep the 4-char indentation, unless the whole file is converted. I'd
> > like to avoid a total reformatting of the codebase, but we could look at
> > it on a file by file base if it becomes relevant.
> 
> Sounds reasonable.
> 

Doesn't to me.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Anthony Liguori]

On 08/22/2010 01:36 PM, malc wrote:
>>>>
>>>> But how would you do that? Drop the CODING_STYLE (and accept
>>>> anything)? Switch to a new CODING_STYLE that is widely appreciated and
>>>> so all bikeshedding will cease? Enforce current style?
>>>>          
>>> I would suggest we either clean up the existing rule, or switch to the
>>> Linux kernel style, with the explicit exemption that existing code can
>>> keep the 4-char indentation, unless the whole file is converted. I'd
>>> like to avoid a total reformatting of the codebase, but we could look at
>>> it on a file by file base if it becomes relevant.
>>>        
>> Sounds reasonable.
>>
>>      
> Doesn't to me.
>    

I'm strongly opposed to any reformatting of the tree.

All it does is break git blame which makes debugging harder without 
offering any real benefits.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/22/2010 09:42 PM, Anthony Liguori wrote:
>
> I'm strongly opposed to any reformatting of the tree.

I strongly second this statement.

>
> All it does is break git blame which makes debugging harder without 
> offering any real benefits.
>

Even worse, it makes backporting patches much harder.

Sometimes we have to refactor the code to introduce new features or fix 
deep deficiencies.  Let's not do it just to change the code style.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Kevin Wolf]

Am 22.08.2010 20:42, schrieb Anthony Liguori:
> On 08/22/2010 01:36 PM, malc wrote:
>>>>>
>>>>> But how would you do that? Drop the CODING_STYLE (and accept
>>>>> anything)? Switch to a new CODING_STYLE that is widely appreciated and
>>>>> so all bikeshedding will cease? Enforce current style?
>>>>>          
>>>> I would suggest we either clean up the existing rule, or switch to the
>>>> Linux kernel style, with the explicit exemption that existing code can
>>>> keep the 4-char indentation, unless the whole file is converted. I'd
>>>> like to avoid a total reformatting of the codebase, but we could look at
>>>> it on a file by file base if it becomes relevant.
>>>>        
>>> Sounds reasonable.
>>>
>>>      
>> Doesn't to me.
>>    
> 
> I'm strongly opposed to any reformatting of the tree.
> 
> All it does is break git blame which makes debugging harder without 
> offering any real benefits.

Thanks, Anthony. I fully agree. (And Avi added another good reason.)

And while I'm posting my hopefully only post to this thread, let me
mention that I'm fine with our current CODING_STYLE and can't see a
reason for another switch.

Kevin

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/22/10 20:42, Anthony Liguori wrote:
> On 08/22/2010 01:36 PM, malc wrote:
>>>>>
>>>>> But how would you do that? Drop the CODING_STYLE (and accept
>>>>> anything)? Switch to a new CODING_STYLE that is widely appreciated and
>>>>> so all bikeshedding will cease? Enforce current style?
>>>>>          
>>>> I would suggest we either clean up the existing rule, or switch to the
>>>> Linux kernel style, with the explicit exemption that existing code can
>>>> keep the 4-char indentation, unless the whole file is converted. I'd
>>>> like to avoid a total reformatting of the codebase, but we could
>>>> look at
>>>> it on a file by file base if it becomes relevant.
>>>>        
>>> Sounds reasonable.
>>>    
>> Doesn't to me.
> 
> I'm strongly opposed to any reformatting of the tree.
> 
> All it does is break git blame which makes debugging harder without
> offering any real benefits.

Well my point was to avoid reformatting things, which is why we went for
the kernel style, but kept 4 space indentations, we would pretty much
get there without any reformatting.

Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 08/22/2010 01:36 PM, malc wrote:
>>>>>
>>>>> But how would you do that? Drop the CODING_STYLE (and accept
>>>>> anything)? Switch to a new CODING_STYLE that is widely appreciated and
>>>>> so all bikeshedding will cease? Enforce current style?
>>>>>          
>>>> I would suggest we either clean up the existing rule, or switch to the
>>>> Linux kernel style, with the explicit exemption that existing code can
>>>> keep the 4-char indentation, unless the whole file is converted. I'd
>>>> like to avoid a total reformatting of the codebase, but we could look at
>>>> it on a file by file base if it becomes relevant.
>>>>        
>>> Sounds reasonable.
>>>
>>>      
>> Doesn't to me.
>>    
>
> I'm strongly opposed to any reformatting of the tree.
>
> All it does is break git blame which makes debugging harder without
> offering any real benefits.

And that's why enforcing the parts of the coding style that are
insufficiently consistent with the existing code (mandatory braces, for
instance) is a waste of time.  Writing our own tools to help with that
is an even bigger waste of time.

Everyone's free to waste his or her time however he or she pleases, of
course.  But I respectfully request to refrain from wasting somebody
else's time.  Yes, we should insist people make their changes blend with
the existing code.  But making them jump through additional brace-hoops
when their changes do blend crosses the line for me.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Anthony Liguori]

On 08/22/2010 11:49 AM, Jes Sorensen wrote:
>>> 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.
>>>        
>> But how would you do that? Drop the CODING_STYLE (and accept
>> anything)? Switch to a new CODING_STYLE that is widely appreciated and
>> so all bikeshedding will cease? Enforce current style?
>>      
> I would suggest we either clean up the existing rule, or switch to the
> Linux kernel style, with the explicit exemption that existing code can
> keep the 4-char indentation, unless the whole file is converted. I'd
> like to avoid a total reformatting of the codebase, but we could look at
> it on a file by file base if it becomes relevant.
>    

Why is this even still being discussed?  What problem are people 
actually trying to solve?

Can someone point to a bug in QEMU that's been caused because of 
CODING_STYLE or the fact that some patches don't adhere to it?

I don't see a problem with the way things are today.

Regards,

Anthony Liguori

> Regards,
> Jes
>    

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sun, Aug 22, 2010 at 6:41 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 08/22/2010 11:49 AM, Jes Sorensen wrote:
>>>>
>>>> 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.
>>>>
>>>
>>> But how would you do that? Drop the CODING_STYLE (and accept
>>> anything)? Switch to a new CODING_STYLE that is widely appreciated and
>>> so all bikeshedding will cease? Enforce current style?
>>>
>>
>> I would suggest we either clean up the existing rule, or switch to the
>> Linux kernel style, with the explicit exemption that existing code can
>> keep the 4-char indentation, unless the whole file is converted. I'd
>> like to avoid a total reformatting of the codebase, but we could look at
>> it on a file by file base if it becomes relevant.
>>
>
> Why is this even still being discussed?  What problem are people actually
> trying to solve?
>
> Can someone point to a bug in QEMU that's been caused because of
> CODING_STYLE or the fact that some patches don't adhere to it?

7b1df88f284f462ecb236931ad863a815f243195

> I don't see a problem with the way things are today.

There is the problem that some patch submitters are reminded of
CODING_STYLE while others aren't. Some don't need to be reminded but
they are not part of the problem.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Anthony Liguori]

On 08/22/2010 01:56 PM, Blue Swirl wrote:
>> Why is this even still being discussed?  What problem are people actually
>> trying to solve?
>>
>> Can someone point to a bug in QEMU that's been caused because of
>> CODING_STYLE or the fact that some patches don't adhere to it?
>>      
> 7b1df88f284f462ecb236931ad863a815f243195
>    

That's a hell of a bug :-)

>> I don't see a problem with the way things are today.
>>      
> There is the problem that some patch submitters are reminded of
> CODING_STYLE while others aren't.

I still think we spend far too much time discussing this on list.  While 
I stand corrected that we've ever had a bug because of this, I have a 
hard time believing this is anywhere in the top 10 list of issues that 
we have.

Regards,

Anthony Liguori

>   Some don't need to be reminded but
> they are not part of the problem.
>    

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[malc]

On Sun, 22 Aug 2010, Anthony Liguori wrote:

> On 08/22/2010 01:56 PM, Blue Swirl wrote:
> > > Why is this even still being discussed?  What problem are people actually
> > > trying to solve?
> > > 
> > > Can someone point to a bug in QEMU that's been caused because of
> > > CODING_STYLE or the fact that some patches don't adhere to it?
> > >      
> > 7b1df88f284f462ecb236931ad863a815f243195
> >    
> 
> That's a hell of a bug :-)
> 
> > > I don't see a problem with the way things are today.
> > >      
> > There is the problem that some patch submitters are reminded of
> > CODING_STYLE while others aren't.
> 
> I still think we spend far too much time discussing this on list.  While I
> stand corrected that we've ever had a bug because of this, I have a hard time
> believing this is anywhere in the top 10 list of issues that we have.

You don't.

if (len != 4) {
   /* TODO: Signal an error? */
}

Is just as affected and follows the style perfectly.

> 
> Regards,
> 
> Anthony Liguori
> 
> >   Some don't need to be reminded but
> > they are not part of the problem.
> >    

You weirdly snipped that part, here it is in full:

 
> There is the problem that some patch submitters are reminded of
> CODING_STYLE while others aren't. Some don't need to be reminded but
> they are not part of the problem.

And to this i fully subscribe. 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sun, Aug 22, 2010 at 7:44 PM, malc <av1474@comtv.ru> wrote:
> On Sun, 22 Aug 2010, Anthony Liguori wrote:
>
>> On 08/22/2010 01:56 PM, Blue Swirl wrote:
>> > > Why is this even still being discussed?  What problem are people actually
>> > > trying to solve?
>> > >
>> > > Can someone point to a bug in QEMU that's been caused because of
>> > > CODING_STYLE or the fact that some patches don't adhere to it?
>> > >
>> > 7b1df88f284f462ecb236931ad863a815f243195
>> >
>>
>> That's a hell of a bug :-)
>>
>> > > I don't see a problem with the way things are today.
>> > >
>> > There is the problem that some patch submitters are reminded of
>> > CODING_STYLE while others aren't.
>>
>> I still think we spend far too much time discussing this on list.  While I
>> stand corrected that we've ever had a bug because of this, I have a hard time
>> believing this is anywhere in the top 10 list of issues that we have.
>
> You don't.
>
> if (len != 4) {
>   /* TODO: Signal an error? */
> }
>
> Is just as affected and follows the style perfectly.

No. I think you missed a very, very important detail in the line with
the comment.

>>
>> Regards,
>>
>> Anthony Liguori
>>
>> >   Some don't need to be reminded but
>> > they are not part of the problem.
>> >
>
> You weirdly snipped that part, here it is in full:
>
>
>> There is the problem that some patch submitters are reminded of
>> CODING_STYLE while others aren't. Some don't need to be reminded but
>> they are not part of the problem.
>
> And to this i fully subscribe.
>
> --
> mailto:av1474@comtv.ru
>

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1466 bytes --]

On Sun, 22 Aug 2010, Blue Swirl wrote:

> On Sun, Aug 22, 2010 at 7:44 PM, malc <av1474@comtv.ru> wrote:
> > On Sun, 22 Aug 2010, Anthony Liguori wrote:
> >
> >> On 08/22/2010 01:56 PM, Blue Swirl wrote:
> >> > > Why is this even still being discussed?  What problem are people actually
> >> > > trying to solve?
> >> > >
> >> > > Can someone point to a bug in QEMU that's been caused because of
> >> > > CODING_STYLE or the fact that some patches don't adhere to it?
> >> > >
> >> > 7b1df88f284f462ecb236931ad863a815f243195
> >> >
> >>
> >> That's a hell of a bug :-)
> >>
> >> > > I don't see a problem with the way things are today.
> >> > >
> >> > There is the problem that some patch submitters are reminded of
> >> > CODING_STYLE while others aren't.
> >>
> >> I still think we spend far too much time discussing this on list.  While I
> >> stand corrected that we've ever had a bug because of this, I have a hard time
> >> believing this is anywhere in the top 10 list of issues that we have.
> >
> > You don't.
> >
> > if (len != 4) {
> >   /* TODO: Signal an error? */
> > }
> >
> > Is just as affected and follows the style perfectly.
> 
> No. I think you missed a very, very important detail in the line with
> the comment.

The trailing ';'? If so, then no i haven't missed it, i was warned about
it ages ago while playing around with clang.

if (len != 4); === if (len != 4) {}
as far as abstract machine goes.

[..snip..]

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/22/2010 10:44 PM, malc wrote:
>
>> There is the problem that some patch submitters are reminded of
>> CODING_STYLE while others aren't. Some don't need to be reminded but
>> they are not part of the problem.
> And to this i fully subscribe.

I agree.

 From my Linux experience, I started out whining about each and every 
coding style violation.  To my own great self-annoyance.  After a while 
all contributors adjusted and now this is quite rare.

Enforce the coding style with an iron hand, and pretty soon you won't 
have to.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/22/10 22:39, Avi Kivity wrote:
>  On 08/22/2010 10:44 PM, malc wrote:
>>
>>> There is the problem that some patch submitters are reminded of
>>> CODING_STYLE while others aren't. Some don't need to be reminded but
>>> they are not part of the problem.
>> And to this i fully subscribe.
> 
> I agree.
> 
> From my Linux experience, I started out whining about each and every
> coding style violation.  To my own great self-annoyance.  After a while
> all contributors adjusted and now this is quite rare.
> 
> Enforce the coding style with an iron hand, and pretty soon you won't
> have to.

Well with the inconsistency we have now, what is the right iron fist to
apply? Demand the code is consistent with the file you edit or that it's
consistent with whats in CODING_STYLE, even if it means that what you
apply is completely different to the rest of the file?

That is the part I think needs to be decided upon in all of this.

Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/23/2010 04:55 PM, Jes Sorensen wrote:
>
> Well with the inconsistency we have now, what is the right iron fist to
> apply? Demand the code is consistent with the file you edit or that it's
> consistent with whats in CODING_STYLE, even if it means that what you
> apply is completely different to the rest of the file?
>
> That is the part I think needs to be decided upon in all of this.

Patch lines that start with ^\+ should be consistent with CODING_STYLE.  
The maintainers may allow exceptions in certain cases, but contributors 
shouldn't expect this.  Use common sense where available.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/23/10 16:03, Avi Kivity wrote:
>  On 08/23/2010 04:55 PM, Jes Sorensen wrote:
>>
>> Well with the inconsistency we have now, what is the right iron fist to
>> apply? Demand the code is consistent with the file you edit or that it's
>> consistent with whats in CODING_STYLE, even if it means that what you
>> apply is completely different to the rest of the file?
>>
>> That is the part I think needs to be decided upon in all of this.
> 
> Patch lines that start with ^\+ should be consistent with CODING_STYLE. 
> The maintainers may allow exceptions in certain cases, but contributors
> shouldn't expect this.  Use common sense where available.
> 

Right, in my book common sense is to be consistent with the file I am
editing, as long as the file isn't in gross violation of CODING_STYLE,
but maybe my sense just isn't that good.

Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/23/2010 05:07 PM, Jes Sorensen wrote:
> On 08/23/10 16:03, Avi Kivity wrote:
>>   On 08/23/2010 04:55 PM, Jes Sorensen wrote:
>>> Well with the inconsistency we have now, what is the right iron fist to
>>> apply? Demand the code is consistent with the file you edit or that it's
>>> consistent with whats in CODING_STYLE, even if it means that what you
>>> apply is completely different to the rest of the file?
>>>
>>> That is the part I think needs to be decided upon in all of this.
>> Patch lines that start with ^\+ should be consistent with CODING_STYLE.
>> The maintainers may allow exceptions in certain cases, but contributors
>> shouldn't expect this.  Use common sense where available.
>>
> Right, in my book common sense is to be consistent with the file I am
> editing, as long as the file isn't in gross violation of CODING_STYLE,
> but maybe my sense just isn't that good.

I'd have said the opposite - if the file _is_ in gross violation, mixing 
styles would make it unreadable.  If it's just slightly different then 
using C_S would improve it incrementally.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sun, Aug 22, 2010 at 7:28 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 08/22/2010 01:56 PM, Blue Swirl wrote:
>>>
>>> Why is this even still being discussed?  What problem are people actually
>>> trying to solve?
>>>
>>> Can someone point to a bug in QEMU that's been caused because of
>>> CODING_STYLE or the fact that some patches don't adhere to it?
>>>
>>
>> 7b1df88f284f462ecb236931ad863a815f243195
>>
>
> That's a hell of a bug :-)

Yeah. IIRC I almost dismissed the warning from clang as false positive
because I just couldn't see any bug.

>>> I don't see a problem with the way things are today.
>>>
>>
>> There is the problem that some patch submitters are reminded of
>> CODING_STYLE while others aren't.
>
> I still think we spend far too much time discussing this on list.  While I
> stand corrected that we've ever had a bug because of this, I have a hard
> time believing this is anywhere in the top 10 list of issues that we have.

When an outsider submits a patch which gets reviewed, CODING_STYLE
issues can almost certainly be expected. The other frequent issues
(for example memory management) should be addressed by HACKING.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/22/2010 09:56 PM, Blue Swirl wrote:
>
>> Can someone point to a bug in QEMU that's been caused because of
>> CODING_STYLE or the fact that some patches don't adhere to it?
> 7b1df88f284f462ecb236931ad863a815f243195

How was this bug caused by CODING_STYLE?  In fact, if CODING_STYLE was 
applied correctly then this bug would stand out much more:

     if (...) {
         /* .... */;
     }
         return;

or be completely eliminated:

     if (...) {
         /* .... */;
         return;
     }


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sun, Aug 22, 2010 at 8:09 PM, Avi Kivity <avi@redhat.com> wrote:
>  On 08/22/2010 09:56 PM, Blue Swirl wrote:
>>
>>> Can someone point to a bug in QEMU that's been caused because of
>>> CODING_STYLE or the fact that some patches don't adhere to it?
>>
>> 7b1df88f284f462ecb236931ad863a815f243195
>
> How was this bug caused by CODING_STYLE?  In fact, if CODING_STYLE was
> applied correctly then this bug would stand out much more:
>
>    if (...) {
>        /* .... */;
>    }
>        return;
>
> or be completely eliminated:
>
>    if (...) {
>        /* .... */;
>        return;
>    }

Exactly. It was caused by code not adhering to CODING_STYLE (maybe
because it didn't exist at the time).

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Anthony Liguori]

On 08/22/2010 03:09 PM, Avi Kivity wrote:
>  On 08/22/2010 09:56 PM, Blue Swirl wrote:
>>
>>> Can someone point to a bug in QEMU that's been caused because of
>>> CODING_STYLE or the fact that some patches don't adhere to it?
>> 7b1df88f284f462ecb236931ad863a815f243195
>
> How was this bug caused by CODING_STYLE?  In fact, if CODING_STYLE was 
> applied correctly then this bug would stand out much more:
>
>     if (...) {
>         /* .... */;
>     }
>         return;

"or the fact that some patches don't adhere to it".  If CODING_STYLE was 
enforced at commit time, this bug would not have happened most likely.

But it's such an odd case that I'd say it's just the exception that 
proves the rule that a loose CODING_STYLE isn't significantly impacting 
overall quality.

I also don't believe that it's creating distress in contributors.  I 
believe timely patch review/commit is probably a more important issue 
for contributors than whether some people get by without CODING_STYLE 
being strictly enforced.

Regards,

Anthony Liguori

>
> or be completely eliminated:
>
>     if (...) {
>         /* .... */;
>         return;
>     }
>
>

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/22/2010 11:17 PM, Anthony Liguori wrote:
> On 08/22/2010 03:09 PM, Avi Kivity wrote:
>>  On 08/22/2010 09:56 PM, Blue Swirl wrote:
>>>
>>>> Can someone point to a bug in QEMU that's been caused because of
>>>> CODING_STYLE or the fact that some patches don't adhere to it?
>>> 7b1df88f284f462ecb236931ad863a815f243195
>>
>> How was this bug caused by CODING_STYLE?  In fact, if CODING_STYLE 
>> was applied correctly then this bug would stand out much more:
>>
>>     if (...) {
>>         /* .... */;
>>     }
>>         return;
>
> "or the fact that some patches don't adhere to it".  If CODING_STYLE 
> was enforced at commit time, this bug would not have happened most 
> likely.

Ah, yes.

>
> But it's such an odd case that I'd say it's just the exception that 
> proves the rule that a loose CODING_STYLE isn't significantly 
> impacting overall quality.

I agree.  IMO a consistent coding style is mainly an aid to the reader, 
the brain has one less thing to disentangle.

> I also don't believe that it's creating distress in contributors.  I 
> believe timely patch review/commit is probably a more important issue 
> for contributors than whether some people get by without CODING_STYLE 
> being strictly enforced.

Definitely.  Especially as people can adjust to CODING_STYLE.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

[Qemu-devel] Re: [PATCH 0/5] CODING_STYLE amendments

[Paolo Bonzini]

On 08/22/2010 08:56 PM, Blue Swirl wrote:
> On Sun, Aug 22, 2010 at 6:41 PM, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> On 08/22/2010 11:49 AM, Jes Sorensen wrote:
>>>>>
>>>>> 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.
>>>>>
>>>>
>>>> But how would you do that? Drop the CODING_STYLE (and accept
>>>> anything)? Switch to a new CODING_STYLE that is widely appreciated and
>>>> so all bikeshedding will cease? Enforce current style?
>>>>
>>>
>>> I would suggest we either clean up the existing rule, or switch to the
>>> Linux kernel style, with the explicit exemption that existing code can
>>> keep the 4-char indentation, unless the whole file is converted. I'd
>>> like to avoid a total reformatting of the codebase, but we could look at
>>> it on a file by file base if it becomes relevant.
>>>
>>
>> Why is this even still being discussed?  What problem are people actually
>> trying to solve?
>>
>> Can someone point to a bug in QEMU that's been caused because of
>> CODING_STYLE or the fact that some patches don't adhere to it?
>
> 7b1df88f284f462ecb236931ad863a815f243195

This would have been caught just as well by -Wunreachable-code.  We 
don't enable it and GCC manual discourages it, but it would be 
worthwhile checking how many false positives it gives in QEMU.

I'm also quite surprised that the QEMU coding standards allow 
return-with-value when the value is void:

     if (hdr->data[0] & 1) {
         if (len != 4)
             /* TODO: Signal an error? */;
             return;

          return l2cap_sframe_in(ch, le16_to_cpup((void *) hdr->data));
     }

Paolo

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/21/10 12:47, Blue Swirl wrote:
> On Sat, Aug 21, 2010 at 9:54 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> 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.

Problem with mass conversion is that it becomes really hard to track
changes for debugging. While it would be nice to get all code to look
the 'right way<tm>' in a snap, then I think it will cause more harm than
good.

>> 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.

Yes, it's a classic case, there is always the historical side and
personal bios for why it was written the way it is. Often this is goes
back to personal preference rather than reason :( IMHO it isn't such a
big issue what the style is, as long as it is consistent and efficient.
The problem with the style we have now is that is is totally
inconsistent and has elements making it harder to debug the code, like
the braces around single line if statements. I totally agree with Markus
that it seems like wasted effort to come up with new tools and having to
maintain them when there are good ones out there like the ones from the
Linux kernel.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sun, Aug 22, 2010 at 4:40 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 08/21/10 12:47, Blue Swirl wrote:
>> On Sat, Aug 21, 2010 at 9:54 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>> 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.
>
> Problem with mass conversion is that it becomes really hard to track
> changes for debugging. While it would be nice to get all code to look
> the 'right way<tm>' in a snap, then I think it will cause more harm than
> good.

Well, consider for example mass braces conversion to the One Style,
whichever that is. Would it be better to do it in one commit or
multiple commits? If the latter, push all commits back-to-back or just
one at a time now and then?

At the extreme end, we could even convert one statement per commit.
This would make bug hunting with git bisect extremely precise. There
would be a cost of long commit log.

For the patch submitters, wouldn't one shot conversion (with one push,
one or many commits) be less painful?

>>> 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.
>
> Yes, it's a classic case, there is always the historical side and
> personal bios for why it was written the way it is. Often this is goes
> back to personal preference rather than reason :( IMHO it isn't such a
> big issue what the style is, as long as it is consistent and efficient.
> The problem with the style we have now is that is is totally
> inconsistent

Fully agree.

> and has elements making it harder to debug the code, like
> the braces around single line if statements.

What's the problem?

> I totally agree with Markus
> that it seems like wasted effort to come up with new tools and having to
> maintain them when there are good ones out there like the ones from the
> Linux kernel.

I also find the tool argument very attractive. No other style has that benefit.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2866 bytes --]

On Sun, 22 Aug 2010, Blue Swirl wrote:

> On Sun, Aug 22, 2010 at 4:40 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> > On 08/21/10 12:47, Blue Swirl wrote:
> >> On Sat, Aug 21, 2010 at 9:54 AM, Markus Armbruster <armbru@redhat.com> wrote:
> >>> 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.
> >
> > Problem with mass conversion is that it becomes really hard to track
> > changes for debugging. While it would be nice to get all code to look
> > the 'right way<tm>' in a snap, then I think it will cause more harm than
> > good.
> 
> Well, consider for example mass braces conversion to the One Style,
> whichever that is. Would it be better to do it in one commit or
> multiple commits? If the latter, push all commits back-to-back or just
> one at a time now and then?
> 
> At the extreme end, we could even convert one statement per commit.
> This would make bug hunting with git bisect extremely precise. There
> would be a cost of long commit log.
> 
> For the patch submitters, wouldn't one shot conversion (with one push,
> one or many commits) be less painful?
> 
> >>> 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.
> >
> > Yes, it's a classic case, there is always the historical side and
> > personal bios for why it was written the way it is. Often this is goes
> > back to personal preference rather than reason :( IMHO it isn't such a
> > big issue what the style is, as long as it is consistent and efficient.
> > The problem with the style we have now is that is is totally
> > inconsistent
> 
> Fully agree.
> 
> > and has elements making it harder to debug the code, like
> > the braces around single line if statements.
> 
> What's the problem?

Disregarding my own stance on the braces, braces around single statement
is actually helpful w.r.t. debugging imaging trying to set a break point
on said singlesttement, plain impossible in following case:

if (a) b;

> 
> > I totally agree with Markus
> > that it seems like wasted effort to come up with new tools and having to
> > maintain them when there are good ones out there like the ones from the
> > Linux kernel.
> 
> I also find the tool argument very attractive. No other style has that 
> benefit.
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/22/10 20:39, malc wrote:
> Disregarding my own stance on the braces, braces around single statement
> is actually helpful w.r.t. debugging imaging trying to set a break point
> on said singlesttement, plain impossible in following case:
> 
> if (a) b;

Oh there is no talk about suggesting we force things onto a single line.
Putting if(foo) bar(); on the same line is just plain wrong, what I
referred to was this:

    if (foo)
        bar();

vs
    if (foo) {
        bar();
    }

If it is part of a multi-block if()/else block yes it's good for
consistency, if it is a one off in the code, it doesn't add any value
IMHO, it is a loss because it wastes space for no reason.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[john cooper]

Jes Sorensen wrote:
> On 08/22/10 20:39, malc wrote:
>   
>> Disregarding my own stance on the braces, braces around single statement
>> is actually helpful w.r.t. debugging imaging trying to set a break point
>> on said singlesttement, plain impossible in following case:
>>
>> if (a) b;
>>     
>
> Oh there is no talk about suggesting we force things onto a single line.
> Putting if(foo) bar(); on the same line is just plain wrong, what I
> referred to was this:
>
>     if (foo)
>         bar();
>
> vs
>     if (foo) {
>         bar();
>     }
>
> If it is part of a multi-block if()/else block yes it's good for
> consistency, if it is a one off in the code, it doesn't add any value
> IMHO, it is a loss because it wastes space for no reason.
>   
About the only benefit I've found for that added syntactic sugar
is simplifying debug somewhat as you aren't falling in/out of the
grammar optimization.

But from a maintainability perspective, where possible I avoid
formatting which dilutes the information content in a code window.
It negatively impacts readability IMHO.

-john

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Jes Sorensen]

On 08/22/10 20:13, Blue Swirl wrote:
> Well, consider for example mass braces conversion to the One Style,
> whichever that is. Would it be better to do it in one commit or
> multiple commits? If the latter, push all commits back-to-back or just
> one at a time now and then?
> 
> At the extreme end, we could even convert one statement per commit.
> This would make bug hunting with git bisect extremely precise. There
> would be a cost of long commit log.
> 
> For the patch submitters, wouldn't one shot conversion (with one push,
> one or many commits) be less painful?

Hi,

Yes, if we do it, I'd say one commit per file/file-set, like hw/irq.[ch]
etc.

>> and has elements making it harder to debug the code, like
>> the braces around single line if statements.
> 
> What's the problem?

It chews up lines for no reason giving you less of an overview of the
code. Obviously there are cases where it makes sense, but having it
around a return statement is just a waste.

>> I totally agree with Markus
>> that it seems like wasted effort to come up with new tools and having to
>> maintain them when there are good ones out there like the ones from the
>> Linux kernel.
> 
> I also find the tool argument very attractive. No other style has that benefit.

Yes, it's a big win.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/22/2010 07:40 PM, Jes Sorensen wrote:
> I totally agree with Markus
> that it seems like wasted effort to come up with new tools and having to
> maintain them when there are good ones out there like the ones from the
> Linux kernel.
>

scripts/Lindent is just a wrapper around indent(1).  We don't need to 
come up with new tools, just come up with the right switches to indent(1).

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Sun, Aug 22, 2010 at 8:12 PM, Avi Kivity <avi@redhat.com> wrote:
>  On 08/22/2010 07:40 PM, Jes Sorensen wrote:
>>
>> I totally agree with Markus
>> that it seems like wasted effort to come up with new tools and having to
>> maintain them when there are good ones out there like the ones from the
>> Linux kernel.
>>
>
> scripts/Lindent is just a wrapper around indent(1).  We don't need to come
> up with new tools, just come up with the right switches to indent(1).

More importantly, we want checkpatch.pl.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/22/2010 11:16 PM, Blue Swirl wrote:
>
>> scripts/Lindent is just a wrapper around indent(1).  We don't need to come
>> up with new tools, just come up with the right switches to indent(1).
> More importantly, we want checkpatch.pl.

That's a bit too obnoxious in my opinion.  I think the main thing is to 
warn about braces and tabs, everything else is either rare or stands out 
better.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Markus Armbruster]

Avi Kivity <avi@redhat.com> writes:

>  On 08/22/2010 07:40 PM, Jes Sorensen wrote:
>> I totally agree with Markus
>> that it seems like wasted effort to come up with new tools and having to
>> maintain them when there are good ones out there like the ones from the
>> Linux kernel.
>>
>
> scripts/Lindent is just a wrapper around indent(1).  We don't need to
> come up with new tools, just come up with the right switches to
> indent(1).

How can indent help effectively with policing the formatting of new
code, in the form of patches, without touching the old code?

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Avi Kivity]

  On 08/23/2010 02:01 PM, Markus Armbruster wrote:
> Avi Kivity<avi@redhat.com>  writes:
>
>>   On 08/22/2010 07:40 PM, Jes Sorensen wrote:
>>> I totally agree with Markus
>>> that it seems like wasted effort to come up with new tools and having to
>>> maintain them when there are good ones out there like the ones from the
>>> Linux kernel.
>>>
>> scripts/Lindent is just a wrapper around indent(1).  We don't need to
>> come up with new tools, just come up with the right switches to
>> indent(1).
> How can indent help effectively with policing the formatting of new
> code, in the form of patches, without touching the old code?

It can't.  We need something like checkpatch (but not checkpatch itself, 
that's much too whiny).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Tue, Aug 17, 2010 at 8:04 AM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 08/13/10 20:02, Blue Swirl wrote:
>> On Fri, Aug 13, 2010 at 3:22 PM, Miguel Di Ciurcio Filho
>> <miguel.filho@gmail.com> wrote:
>>> The existing code that I have touched don't follow the current coding
>>> style guidance, much less all the new recommendations being suggested.
>>>
>>> Although, I do believe that this situation needs to change. If we
>>> agree in a coding style, I would volunteer to be a some kind of
>>> observer to fix and alert people about coding styles mistakes.
>>
>> I fully agree on the need of change and support your excellent idea.
>> There are other ways to solve the problem, but I believe we need more
>> order than more chaos. Perhaps we the QEMU developers should appoint
>> you the Guardian of the CODING_STYLE, and add a rule that no patch
>> shall be committed without your CS-Acked-by line?
>
> I don't think this would ever work, it is begging for trouble relying on
> one person to review all patches for this.

We could have more than one Guardian and also several Vice-Guardians
if there are enough volunteers. Even better, the patch submitters
could do the checking themselves and add a 'CS-Purity-Certified-by'
line.

> While I agree coding style is good since it enforces consistency, there
> are plenty problems with the old rules. For example the rule to demand
> braces around single line in an if statement. It results in more empty
> lines on the screeen, ie lost screen real estate making it harder for
> someone reading the code to get a good overview.

The logic for the braces is explained in CODING_STYLE.

> If we are going to mod the QEMU coding style rules, I strongly recommend
> we look at the Linux kernel rules, while keeping the 4 space
> indentation, rather than trying to adopt things from libvirt.

This was discussed earlier and I suggested switching to Linux kernel
style (without any 4 space exceptions) because of 'indent' support
which would allow for mechanical reformatting. Please visit the list
archives for the discussion and what was the conclusion.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[Blue Swirl]

On Thu, Aug 12, 2010 at 6:56 PM, malc <av1474@comtv.ru> wrote:
> On Thu, 12 Aug 2010, Blue Swirl wrote:
>
>> Add a few rules, based loosely on libvirt HACKING.
>>
>> Blue Swirl (5):
>>   CODING_STYLE: add preprocessor rules
>>   CODING_STYLE: add C type rules
>>   CODING_STYLE: add memory management rules
>>   CODING_STYLE: add string management rules
>>   CODING_STYLE: add rules for printf-like functions
>>
>>  CODING_STYLE |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 111 insertions(+), 0 deletions(-)
>
> While intentions of this are good, i believe this goes too far, i doubt
> that the proposed additions are enforcable and have no doubts that they
> will be widely ignored and at the same time provide more grounds for
> whining.

Then, either we add some kind of enforcement or we downgrade the rules
to recommendations or guidelines.

> Furthermore the existing code doesn't follow them, going out on
> a limb, it's more likely that one would look around the code he/she
> modifies and base his/her modifications on the surrounding code than to
> follow the style that conflicts with it.

I'll try to remove the new stuff.

On the other hand, it should be possible to introduce new rules in
general. We can explicitly state that old code does not follow this
rule, but patches to fix the problems are appreciated and immediately
applied.

This also applies to the unfortunate situation with the braces rule,
which is constantly broken.

Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1667 bytes --]

On Fri, 13 Aug 2010, Blue Swirl wrote:

> On Thu, Aug 12, 2010 at 6:56 PM, malc <av1474@comtv.ru> wrote:
> > On Thu, 12 Aug 2010, Blue Swirl wrote:
> >
> >> Add a few rules, based loosely on libvirt HACKING.
> >>
> >> Blue Swirl (5):
> >>   CODING_STYLE: add preprocessor rules
> >>   CODING_STYLE: add C type rules
> >>   CODING_STYLE: add memory management rules
> >>   CODING_STYLE: add string management rules
> >>   CODING_STYLE: add rules for printf-like functions
> >>
> >>  CODING_STYLE |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 files changed, 111 insertions(+), 0 deletions(-)
> >
> > While intentions of this are good, i believe this goes too far, i doubt
> > that the proposed additions are enforcable and have no doubts that they
> > will be widely ignored and at the same time provide more grounds for
> > whining.
> 
> Then, either we add some kind of enforcement or we downgrade the rules
> to recommendations or guidelines.

I'm fine with the latter.

> 
> > Furthermore the existing code doesn't follow them, going out on
> > a limb, it's more likely that one would look around the code he/she
> > modifies and base his/her modifications on the surrounding code than to
> > follow the style that conflicts with it.
> 
> I'll try to remove the new stuff.
> 
> On the other hand, it should be possible to introduce new rules in
> general. We can explicitly state that old code does not follow this
> rule, but patches to fix the problems are appreciated and immediately
> applied.
> 
> This also applies to the unfortunate situation with the braces rule,
> which is constantly broken.

-- 
mailto:av1474@comtv.ru

[Qemu-devel] Re: [PATCH 0/5] CODING_STYLE amendments

[Paolo Bonzini]

On 08/12/2010 02:56 PM, malc wrote:
> On Thu, 12 Aug 2010, Blue Swirl wrote:
>
>> Add a few rules, based loosely on libvirt HACKING.
>>
>> Blue Swirl (5):
>>    CODING_STYLE: add preprocessor rules
>>    CODING_STYLE: add C type rules
>>    CODING_STYLE: add memory management rules
>>    CODING_STYLE: add string management rules
>>    CODING_STYLE: add rules for printf-like functions
>>
>>   CODING_STYLE |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 111 insertions(+), 0 deletions(-)
>
> While intentions of this are good, i believe this goes too far, i doubt
> that the proposed additions are enforcable and have no doubts that they
> will be widely ignored and at the same time provide more grounds for
> whining. Furthermore the existing code doesn't follow them, going out on
> a limb, it's more likely that one would look around the code he/she
> modifies and base his/her modifications on the surrounding code than to
> follow the style that conflicts with it.

I think this doesn't matter.  The rules Blue Swirl suggested (with a 
couple of exception regarding C types) are mostly dictated by common 
sense and would likely be spotted anyway during review.

In fact, unlike the braces some of them do provide slight advantages in 
terms of code quality and avoiding hidden bugs (e.g. size_t vs. int, 
off_t vs. long).

Even without a "guardian" (whose time would be better spent continuing 
on his QMPification project), I suggest putting in at least patches 
1/3/4/5 now.

Paolo