[Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Andreas Färber]

Disable warnings for spaces before opening parenthesis in
hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: malc <av1474@comtv.ru>
---
 scripts/checkpatch.pl |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8850a5f..5433736 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -246,6 +246,8 @@ our $allowed_asm_includes = qr{(?x:
 )};
 # memory.h: ARM has a custom one
 
+our $audio_files = qr{hw/ac97.c|hw/adlib.c|hw/cs4231a.c|hw/es1370.c|hw/gus.c|hw/sb16.c};
+
 sub build_types {
 	my $mods = "(?x:  \n" . join("|\n  ", @modifierList) . "\n)";
 	my $all = "(?x:  \n" . join("|\n  ", @typeList) . "\n)";
@@ -1966,6 +1968,9 @@ sub process {
 				asm|__asm__)$/x)
 			{
 
+			# malc wants to keep spacing consistent in the audio files.
+			} elsif ($realfile =~ /($audio_files)/) {
+
 			# cpp #define statements have non-optional spaces, ie
 			# if there is a space between the name and the open
 			# parenthesis it is simply not a parameter group.
-- 
1.7.7

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Andreas Färber]

Am 09.02.2012 15:59, schrieb Andreas Färber:
> Disable warnings for spaces before opening parenthesis in
> hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Cc: malc <av1474@comtv.ru>
> ---

For testing I used the following pseudo-patch:

--8<--

--- a/hw/sb16.c
+++ b/hw/sb16.c
@@ -1,1 +1,1 @@
-device_init (foobar)
+type_init (foobar)

--- a/hw/foo.c
+++ b/hw/foo.c
@@ -1,1 +1,1 @@
-device_init (foobar)
+type_init (foobar)

--8<--

Adding a rule to warn about missing space before parenthesis is left as
an exercise for malc.

Andreas

>  scripts/checkpatch.pl |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 8850a5f..5433736 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -246,6 +246,8 @@ our $allowed_asm_includes = qr{(?x:
>  )};
>  # memory.h: ARM has a custom one
>  
> +our $audio_files = qr{hw/ac97.c|hw/adlib.c|hw/cs4231a.c|hw/es1370.c|hw/gus.c|hw/sb16.c};
> +
>  sub build_types {
>  	my $mods = "(?x:  \n" . join("|\n  ", @modifierList) . "\n)";
>  	my $all = "(?x:  \n" . join("|\n  ", @typeList) . "\n)";
> @@ -1966,6 +1968,9 @@ sub process {
>  				asm|__asm__)$/x)
>  			{
>  
> +			# malc wants to keep spacing consistent in the audio files.
> +			} elsif ($realfile =~ /($audio_files)/) {
> +
>  			# cpp #define statements have non-optional spaces, ie
>  			# if there is a space between the name and the open
>  			# parenthesis it is simply not a parameter group.

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[malc]

On Thu, 9 Feb 2012, Andreas F?rber wrote:

> Disable warnings for spaces before opening parenthesis in
> hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.
> 
> Signed-off-by: Andreas F?rber <afaerber@suse.de>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Cc: malc <av1474@comtv.ru>

Thanks.

> ---
>  scripts/checkpatch.pl |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 8850a5f..5433736 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -246,6 +246,8 @@ our $allowed_asm_includes = qr{(?x:
>  )};
>  # memory.h: ARM has a custom one
>  
> +our $audio_files = qr{hw/ac97.c|hw/adlib.c|hw/cs4231a.c|hw/es1370.c|hw/gus.c|hw/sb16.c};

My perl is not up to snuff so i wouldn't know how to add audio/* to the
mix.

> +
>  sub build_types {
>  	my $mods = "(?x:  \n" . join("|\n  ", @modifierList) . "\n)";
>  	my $all = "(?x:  \n" . join("|\n  ", @typeList) . "\n)";
> @@ -1966,6 +1968,9 @@ sub process {
>  				asm|__asm__)$/x)
>  			{
>  
> +			# malc wants to keep spacing consistent in the audio files.
> +			} elsif ($realfile =~ /($audio_files)/) {
> +
>  			# cpp #define statements have non-optional spaces, ie
>  			# if there is a space between the name and the open
>  			# parenthesis it is simply not a parameter group.
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Evgeny Voevodin]

On 02/09/2012 06:59 PM, Andreas Färber wrote:
> Disable warnings for spaces before opening parenthesis in
> hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.

Why audio files are such a special thing?
Isn't it be better to revert a patch that introduced checkpatch.pl errors?

-- 
Kind regards,
Evgeny Voevodin,
Leading Software Engineer,
ASWG, Moscow R&D center, Samsung Electronics
e-mail: e.voevodin@samsung.com

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[malc]

On Fri, 10 Feb 2012, Evgeny Voevodin wrote:

> On 02/09/2012 06:59 PM, Andreas F?rber wrote:
> > Disable warnings for spaces before opening parenthesis in
> > hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.
> 
> Why audio files are such a special thing?

Because they are consistently formatted the way they are.

> Isn't it be better to revert a patch that introduced checkpatch.pl errors?

No. 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Anthony Liguori]

On 02/09/2012 10:02 PM, malc wrote:
> On Fri, 10 Feb 2012, Evgeny Voevodin wrote:
>
>> On 02/09/2012 06:59 PM, Andreas F?rber wrote:
>>> Disable warnings for spaces before opening parenthesis in
>>> hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.
>>
>> Why audio files are such a special thing?
>
> Because they are consistently formatted the way they are.

I personally hate the QEMU Coding Style I dislike inconsistency more than any 
particular style.

So I'm with malc here.  I'd be opposed to introducing a new file that deviated 
from Coding Style but for the ones that already do, I see no reason to convert 
them all at once or make the code deviate from the style it's already using.

>
>> Isn't it be better to revert a patch that introduced checkpatch.pl errors?
>
> No.

Regards,

Anthony Liguori


>

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Blue Swirl]

On Fri, Feb 10, 2012 at 17:47, Anthony Liguori <aliguori@us.ibm.com> wrote:
> On 02/09/2012 10:02 PM, malc wrote:
>>
>> On Fri, 10 Feb 2012, Evgeny Voevodin wrote:
>>
>>> On 02/09/2012 06:59 PM, Andreas F?rber wrote:
>>>>
>>>> Disable warnings for spaces before opening parenthesis in
>>>> hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.
>>>
>>>
>>> Why audio files are such a special thing?
>>
>>
>> Because they are consistently formatted the way they are.
>
>
> I personally hate the QEMU Coding Style I dislike inconsistency more than
> any particular style.

I dislike unclear rules more than inconsistency or coding styles.

> So I'm with malc here.  I'd be opposed to introducing a new file that
> deviated from Coding Style but for the ones that already do, I see no reason
> to convert them all at once or make the code deviate from the style it's
> already using.

I'd make a rule, specify the level of importance and try to stick to
it. I would not oppose global reformatting to GNU style even (which I
hate) if that would be the rule. I don't like laissez faire, but if
that is the rule then fine.

>
>>
>>> Isn't it be better to revert a patch that introduced checkpatch.pl
>>> errors?
>>
>>
>> No.
>
>
> Regards,
>
> Anthony Liguori
>
>
>>
>

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Anthony Liguori]

On 02/11/2012 03:44 AM, Blue Swirl wrote:
> On Fri, Feb 10, 2012 at 17:47, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> On 02/09/2012 10:02 PM, malc wrote:
>>>
>>> On Fri, 10 Feb 2012, Evgeny Voevodin wrote:
>>>
>>>> On 02/09/2012 06:59 PM, Andreas F?rber wrote:
>>>>>
>>>>> Disable warnings for spaces before opening parenthesis in
>>>>> hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.
>>>>
>>>>
>>>> Why audio files are such a special thing?
>>>
>>>
>>> Because they are consistently formatted the way they are.
>>
>>
>> I personally hate the QEMU Coding Style I dislike inconsistency more than
>> any particular style.
>
> I dislike unclear rules more than inconsistency or coding styles.
>
>> So I'm with malc here.  I'd be opposed to introducing a new file that
>> deviated from Coding Style but for the ones that already do, I see no reason
>> to convert them all at once or make the code deviate from the style it's
>> already using.
>
> I'd make a rule, specify the level of importance and try to stick to
> it. I would not oppose global reformatting to GNU style even (which I
> hate) if that would be the rule.

I really hate having these discussions.  I would almost rather we just pay the 
one-time cost of re-indenting so we can stop debating about this.

For folks that feel strongly about this, please submit the following:

An indent command that takes the tree to CODING_STYLE along with a diffstat of 
the end result.

Depending on how bad the diffstat is, we can consider doing this and ending this 
set of arguments once and for all.

Regards,

Anthony Liguori

> I don't like laissez faire, but if
> that is the rule then fine.
>
>>
>>>
>>>> Isn't it be better to revert a patch that introduced checkpatch.pl
>>>> errors?
>>>
>>>
>>> No.
>>
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>
>>>
>>
>
>

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Markus Armbruster]

Anthony Liguori <aliguori@us.ibm.com> writes:

> I really hate having these discussions.  I would almost rather we just
> pay the one-time cost of re-indenting so we can stop debating about
> this.
>
> For folks that feel strongly about this, please submit the following:
>
> An indent command that takes the tree to CODING_STYLE along with a
> diffstat of the end result.
>
> Depending on how bad the diffstat is, we can consider doing this and
> ending this set of arguments once and for all.

The only justification for an idiosyncratic coding style I can buy is
minimizing reindentation of old code.  If we reindent anyway, reindent
to something that isn't specific to the QEMU island, please.

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Anthony Liguori]

On 02/17/2012 08:55 AM, Markus Armbruster wrote:
> Anthony Liguori<aliguori@us.ibm.com>  writes:
>
>> I really hate having these discussions.  I would almost rather we just
>> pay the one-time cost of re-indenting so we can stop debating about
>> this.
>>
>> For folks that feel strongly about this, please submit the following:
>>
>> An indent command that takes the tree to CODING_STYLE along with a
>> diffstat of the end result.
>>
>> Depending on how bad the diffstat is, we can consider doing this and
>> ending this set of arguments once and for all.
>
> The only justification for an idiosyncratic coding style I can buy is
> minimizing reindentation of old code.

Well this was what I was getting at in my previous comments.  If we just need to 
reindent < 10 files with a few random changes here and there, then maybe that 
isn't so bad.

But if we have to touch every single file in the tree in a significant way, then 
no way is it justified.

> If we reindent anyway, reindent
> to something that isn't specific to the QEMU island, please.

I don't even want to consider something that touches every line of code.  That's 
effectively creating a new source tree and losing the continuity of our SCM history.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Blue Swirl]

On Fri, Feb 17, 2012 at 15:26, Anthony Liguori <aliguori@us.ibm.com> wrote:
> On 02/17/2012 08:55 AM, Markus Armbruster wrote:
>>
>> Anthony Liguori<aliguori@us.ibm.com>  writes:
>>
>>> I really hate having these discussions.  I would almost rather we just
>>> pay the one-time cost of re-indenting so we can stop debating about
>>> this.
>>>
>>> For folks that feel strongly about this, please submit the following:
>>>
>>> An indent command that takes the tree to CODING_STYLE along with a
>>> diffstat of the end result.
>>>
>>> Depending on how bad the diffstat is, we can consider doing this and
>>> ending this set of arguments once and for all.
>>
>>
>> The only justification for an idiosyncratic coding style I can buy is
>> minimizing reindentation of old code.
>
>
> Well this was what I was getting at in my previous comments.  If we just
> need to reindent < 10 files with a few random changes here and there, then
> maybe that isn't so bad.
>
> But if we have to touch every single file in the tree in a significant way,
> then no way is it justified.

One way to handle this is gradual reformatting, every time when code
is touched, only changes towards common CODING_STYLE are allowed.
Small, contained reformatting patches should be also allowed, for
example to adjust brace style in one file a time or to remove spaces
at the end of line.

>> If we reindent anyway, reindent
>> to something that isn't specific to the QEMU island, please.
>
>
> I don't even want to consider something that touches every line of code.
>  That's effectively creating a new source tree and losing the continuity of
> our SCM history.

I think only 'git blame' output would be affected and that is not 100%
reliable anyway, considering for example code movement.

> Regards,
>
> Anthony Liguori
>
>

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Stefan Weil]

Am 18.02.2012 10:13, schrieb Blue Swirl:
> On Fri, Feb 17, 2012 at 15:26, Anthony Liguori <aliguori@us.ibm.com> 
> wrote:
>> Well this was what I was getting at in my previous comments.  If we just
>> need to reindent < 10 files with a few random changes here and there, 
>> then
>> maybe that isn't so bad.
>>
>> But if we have to touch every single file in the tree in a 
>> significant way,
>> then no way is it justified.
>
> One way to handle this is gradual reformatting, every time when code
> is touched, only changes towards common CODING_STYLE are allowed.
> Small, contained reformatting patches should be also allowed, for
> example to adjust brace style in one file a time or to remove spaces
> at the end of line.

I'd appreciate it very much if we could remove spaces at line endings
for all non binary files as soon as possible.

Newer versions of git (maybe also older ones with appropriate configuration,
see 'git config --help', core.whitespace / blank-at-eol/ apply.whitespace)
complain about patches which add such spaces. Nevertheless even recent 
patches
did add spaces, so obviously not all committers use that git settings.

It's good practice to use an editor which automatically removes spaces 
at end
of line (I think most editors can be configured to do this). With the 
current
code, this is difficult because it introduces additional whitespace changes
when I edit a file with spaces which are removed by the editor.

>
>>
>> I don't even want to consider something that touches every line of code.
>>  That's effectively creating a new source tree and losing the 
>> continuity of
>> our SCM history.
>
> I think only 'git blame' output would be affected and that is not 100%
> reliable anyway, considering for example code movement.

'git blame' can optionally ignore whitespace changes, so that is not
really a problem. It can even ignore code movements :-)
Just use 'git blame -w -C'.

Regards,
Stefan

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Anthony Liguori]

On 02/18/2012 04:56 AM, Stefan Weil wrote:
> Am 18.02.2012 10:13, schrieb Blue Swirl:
>> On Fri, Feb 17, 2012 at 15:26, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>> Well this was what I was getting at in my previous comments. If we just
>>> need to reindent < 10 files with a few random changes here and there, then
>>> maybe that isn't so bad.
>>>
>>> But if we have to touch every single file in the tree in a significant way,
>>> then no way is it justified.
>>
>> One way to handle this is gradual reformatting, every time when code
>> is touched, only changes towards common CODING_STYLE are allowed.
>> Small, contained reformatting patches should be also allowed, for
>> example to adjust brace style in one file a time or to remove spaces
>> at the end of line.
>
> I'd appreciate it very much if we could remove spaces at line endings
> for all non binary files as soon as possible.

Why?

I really can't understand this.  It's not visible so it does no harm.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Blue Swirl]

On Sat, Feb 18, 2012 at 14:18, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 02/18/2012 04:56 AM, Stefan Weil wrote:
>>
>> Am 18.02.2012 10:13, schrieb Blue Swirl:
>>>
>>> On Fri, Feb 17, 2012 at 15:26, Anthony Liguori <aliguori@us.ibm.com>
>>> wrote:
>>>>
>>>> Well this was what I was getting at in my previous comments. If we just
>>>> need to reindent < 10 files with a few random changes here and there,
>>>> then
>>>> maybe that isn't so bad.
>>>>
>>>> But if we have to touch every single file in the tree in a significant
>>>> way,
>>>> then no way is it justified.
>>>
>>>
>>> One way to handle this is gradual reformatting, every time when code
>>> is touched, only changes towards common CODING_STYLE are allowed.
>>> Small, contained reformatting patches should be also allowed, for
>>> example to adjust brace style in one file a time or to remove spaces
>>> at the end of line.
>>
>>
>> I'd appreciate it very much if we could remove spaces at line endings
>> for all non binary files as soon as possible.
>
>
> Why?
>
> I really can't understand this.  It's not visible so it does no harm.

It's not the end of the world, but they could be mangled (which will
make patches fail to apply) when mailed and they waste precious bytes.
They are also not useful in any way, so better remove them.

> Regards,
>
> Anthony Liguori

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Andreas Färber]

Am 18.02.2012 10:13, schrieb Blue Swirl:
> One way to handle this is gradual reformatting, every time when code
> is touched, only changes towards common CODING_STYLE are allowed.

That's what we've been doing wrt braces and I appreciate us not
cluttering the history with reformatting commits.

Especially at a time where I'm touching every target (and seeing quite
some variations on coding style) I'm not so happy about this initiative.

> On Fri, Feb 17, 2012 at 15:26, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> I don't even want to consider something that touches every line of code.
>>  That's effectively creating a new source tree and losing the continuity of
>> our SCM history.
> 
> I think only 'git blame' output would be affected and that is not 100%
> reliable anyway, considering for example code movement.

I use repo.or.cz's blame function quite often to find out who to cc or
what commit to mention, and (valid) typo fixes are already troublesome,
requiring to go to the parent commit and to navigate to the same file in
its tree. One cannot specify any custom ignore options there.

Is there a better tool to do such interactive, recursive git-blame?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]

On 02/18/2012 08:53 AM, Andreas Färber wrote:

>> I think only 'git blame' output would be affected and that is not 100%
>> reliable anyway, considering for example code movement.
> 
> I use repo.or.cz's blame function quite often to find out who to cc or
> what commit to mention, and (valid) typo fixes are already troublesome,
> requiring to go to the parent commit and to navigate to the same file in
> its tree. One cannot specify any custom ignore options there.
> 
> Is there a better tool to do such interactive, recursive git-blame?

I use 'git gui blame $commit $file &' for interactive recursive blaming.
 When you find the line that you are interested in blaming, you can
click on the commit link on the left side to see the context at the time
of the commit that introduced the state of that line, then right click
on the right side (the line in question) to easily trigger a 'blame
parent' operation.  It's about the only thing that I use 'git gui' for,
but I find it well worth the pain of using a tcl frontend.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Blue Swirl]

On Fri, Feb 17, 2012 at 14:31, Anthony Liguori <aliguori@us.ibm.com> wrote:
> On 02/11/2012 03:44 AM, Blue Swirl wrote:
>>
>> On Fri, Feb 10, 2012 at 17:47, Anthony Liguori<aliguori@us.ibm.com>
>>  wrote:
>>>
>>> On 02/09/2012 10:02 PM, malc wrote:
>>>>
>>>>
>>>> On Fri, 10 Feb 2012, Evgeny Voevodin wrote:
>>>>
>>>>> On 02/09/2012 06:59 PM, Andreas F?rber wrote:
>>>>>>
>>>>>>
>>>>>> Disable warnings for spaces before opening parenthesis in
>>>>>> hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.
>>>>>
>>>>>
>>>>>
>>>>> Why audio files are such a special thing?
>>>>
>>>>
>>>>
>>>> Because they are consistently formatted the way they are.
>>>
>>>
>>>
>>> I personally hate the QEMU Coding Style I dislike inconsistency more than
>>> any particular style.
>>
>>
>> I dislike unclear rules more than inconsistency or coding styles.
>>
>>> So I'm with malc here.  I'd be opposed to introducing a new file that
>>> deviated from Coding Style but for the ones that already do, I see no
>>> reason
>>> to convert them all at once or make the code deviate from the style it's
>>> already using.
>>
>>
>> I'd make a rule, specify the level of importance and try to stick to
>> it. I would not oppose global reformatting to GNU style even (which I
>> hate) if that would be the rule.
>
>
> I really hate having these discussions.  I would almost rather we just pay
> the one-time cost of re-indenting so we can stop debating about this.

Fully agree on both.

> For folks that feel strongly about this, please submit the following:
>
> An indent command that takes the tree to CODING_STYLE along with a diffstat
> of the end result.
>
> Depending on how bad the diffstat is, we can consider doing this and ending
> this set of arguments once and for all.

IIRC GNU indent could not be convinced to follow QEMU style. A quick
test on target-sparc with the command
indent -kr -i4 -nut -sob -l80 -ss -ncs -il0 -cp1 -nfca -TCPUState
-TCPUSPARCState *.[ch]
is very close, but I couldn't avoid comment reformatting despite
-nfca,  indent is totally confused with helper.h and some casts get
spaces despite -ncs.

> Regards,
>
> Anthony Liguori
>
>
>> I don't like laissez faire, but if
>> that is the rule then fine.
>>
>>>
>>>>
>>>>> Isn't it be better to revert a patch that introduced checkpatch.pl
>>>>> errors?
>>>>
>>>>
>>>>
>>>> No.
>>>
>>>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>
>>>>
>>>
>>
>>
>

[Qemu-devel] [PATCH] astyle: Formatting rules for QEMU

[Stefan Weil]

These AStyle rules try to implement the QEMU coding style.

AStyle can also fix only certain aspects of the coding style,
for example indentation.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

Hi Blue,

maybe this AStyle rules work better. I tried them with target-sparc,
and the result does look quite good.

Regards,
Stefan

 scripts/astylerc |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
 create mode 100644 scripts/astylerc

diff --git a/scripts/astylerc b/scripts/astylerc
new file mode 100644
index 0000000..dfbfd41
--- /dev/null
+++ b/scripts/astylerc
@@ -0,0 +1,19 @@
+# Artistic Style (astyle) options for qemu source code.
+
+# Usage:
+# astyle --options=scripts/astylerc {source files}
+
+# For best results, use latest astyle from http://astyle.sourceforge.net/.
+
+add-brackets
+align-pointer=name
+convert-tabs
+max-code-length=80
+style=otbs
+brackets=linux
+indent=spaces=4
+max-code-length=80
+max-instatement-indent=60
+pad-oper
+pad-header
+unpad-paren
-- 
1.7.9

Re: [Qemu-devel] [PATCH] astyle: Formatting rules for QEMU

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 2022 bytes --]

On Sat, Feb 18, 2012 at 09:07, Stefan Weil <sw@weilnetz.de> wrote:
> These AStyle rules try to implement the QEMU coding style.
>
> AStyle can also fix only certain aspects of the coding style,
> for example indentation.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>
> Hi Blue,
>
> maybe this AStyle rules work better. I tried them with target-sparc,
> and the result does look quite good.

There are still indentation problems when expressions continue to next
line, also this does not look OK:
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -36,7 +36,7 @@

 #define DYNAMIC_PC  1 /* dynamic pc value */
 #define JUMP_PC     2 /* dynamic pc value which takes only two values
-                         according to jump_pc[T2] */
+according to jump_pc[T2] */

Moving the brace to same line as switch case should not be necessary.

Overall astyle seems to be a better tool than GNU indent.

I also tried to use Emacs to perform the indenting with the attached
scripts, but the result is not that great either, especially some
macros and helper.h confuse indentation.

> Regards,
> Stefan
>
>  scripts/astylerc |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
>  create mode 100644 scripts/astylerc
>
> diff --git a/scripts/astylerc b/scripts/astylerc
> new file mode 100644
> index 0000000..dfbfd41
> --- /dev/null
> +++ b/scripts/astylerc
> @@ -0,0 +1,19 @@
> +# Artistic Style (astyle) options for qemu source code.
> +
> +# Usage:
> +# astyle --options=scripts/astylerc {source files}
> +
> +# For best results, use latest astyle from http://astyle.sourceforge.net/.
> +
> +add-brackets
> +align-pointer=name
> +convert-tabs
> +max-code-length=80

This is only supported by the SVN HEAD, not even the download tarball offered.

> +style=otbs
> +brackets=linux
> +indent=spaces=4
> +max-code-length=80
> +max-instatement-indent=60
> +pad-oper
> +pad-header
> +unpad-paren
> --
> 1.7.9
>

[-- Attachment #2: emacs_format_file --]
[-- Type: application/octet-stream, Size: 766 bytes --]

#!/bin/sh
# File: my-indent
# Opens a set of files in emacs and executes the emacs-format-function.
# Assumes the function named emacs-format-function is defined in the
# file named emacs-format-file.

if [ $# == 0 ]
then
   echo "$0 requires at least one argument." 1>&2
   echo "Usage: $0 files-to-indent" 1>&2
   exit 1
fi
while [ $# -ge 1 ]
do
   if [ -d $1 ]
   then
      echo "Argument of $0 $1 cannot be a directory." 1>&2
      exit 1
   fi
   # Check for existence of file:
   ls $1 2> /dev/null | grep $1 > /dev/null
   if [ $? != 0 ]
   then
      echo "$0: $1 not found." 1>&2
      exit 1
   fi
   echo "Indenting $1 with emacs in batch mode"
   xemacs -batch $1 -l ~/script/emacs_format_file.el -f emacs-format-function
   echo
   shift 1
done
exit 0

[-- Attachment #3: emacs_format_file.el --]
[-- Type: application/octet-stream, Size: 323 bytes --]

;;; File: emacs-format-file
;;; Stan Warford
;;; 17 May 2006

(defun emacs-format-function ()
  "Format the whole buffer."
  (set-variable 'c-basic-offset 4)
  (setq-default indent-tabs-mode nil)
  '(indent-tabs-mode nil)
  (indent-region (point-min) (point-max) nil)
  (untabify (point-min) (point-max))
  (save-buffer)
)

Re: [Qemu-devel] [PATCH] astyle: Formatting rules for QEMU

[Stefan Weil]

Am 18.02.2012 11:10, schrieb Blue Swirl:
> There are still indentation problems when expressions continue to next
> line, also this does not look OK:
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -36,7 +36,7 @@
>
> #define DYNAMIC_PC 1 /* dynamic pc value */
> #define JUMP_PC 2 /* dynamic pc value which takes only two values
> - according to jump_pc[T2] */
> +according to jump_pc[T2] */

Writing comments like this looks nice, but needs more work when
the comments are written. Maintenance of the code is also more
difficult: any time a new line with a longer name is added, you
have to reformat all other lines to preserve the good look.

Just add this line to the code given above to see what I mean:
     #define ANY_LONG_PC 3 /* just an example */

This is why I usually write comments in an extra line before the
code statement.

Regards,
Stefan

Re: [Qemu-devel] [PATCH] checkpatch: Don't WARN about missing spaces in audio files

[Blue Swirl]

On Thu, Feb 9, 2012 at 14:59, Andreas Färber <afaerber@suse.de> wrote:
> Disable warnings for spaces before opening parenthesis in
> hw/{ac97,adlib,cs4231a,es1370,gus,sb16}.c.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Cc: malc <av1474@comtv.ru>
> ---
>  scripts/checkpatch.pl |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 8850a5f..5433736 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -246,6 +246,8 @@ our $allowed_asm_includes = qr{(?x:
>  )};
>  # memory.h: ARM has a custom one
>
> +our $audio_files = qr{hw/ac97.c|hw/adlib.c|hw/cs4231a.c|hw/es1370.c|hw/gus.c|hw/sb16.c};
> +
>  sub build_types {
>        my $mods = "(?x:  \n" . join("|\n  ", @modifierList) . "\n)";
>        my $all = "(?x:  \n" . join("|\n  ", @typeList) . "\n)";
> @@ -1966,6 +1968,9 @@ sub process {
>                                asm|__asm__)$/x)
>                        {
>
> +                       # malc wants to keep spacing consistent in the audio files.
> +                       } elsif ($realfile =~ /($audio_files)/) {
> +
>                        # cpp #define statements have non-optional spaces, ie
>                        # if there is a space between the name and the open
>                        # parenthesis it is simply not a parameter group.
> --
> 1.7.7
>

NACK to such special casing.