[Qemu-devel] This patch hunk will hang scripts/checkpatch.pl...

[Qemu-devel] This patch hunk will hang scripts/checkpatch.pl...

[Peter Maydell]

The following patch file (a cut down lump of an actual change
I was making) seems to cause scripts/checkpatch.pl to hang.
I find that script too opaque to try to debug, but I post the
offending input here in case anybody else wishes to investigate.

====begin====
commit 4b3cf48c7fa455eb3b31dc844c959964e750a025
Author: Peter Maydell <peter.maydell@linaro.org>
Date:   Mon Feb 2 18:47:16 2015 +0000

    This patch will hang checkpatch.pl.

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 1110a7e..ab6a93c 100644
@@ -7218,13 +7325,13 @@ static inline int float ## s ##
_compare_internal(float ## s a, float ## s b,\
                                                                              \
 int float ## s ## _compare(float ## s a, float ## s b, float_status *status) \
 {                                                                            \
-    return float ## s ## _compare_internal(a, b, 0 STATUS_VAR);              \
+    return float ## s ## _compare_internal(a, b, 0, status);              \
 }                                                                            \
                                                                              \
 int float ## s ## _compare_quiet(float ## s a, float ## s b,                 \
                                  float_status *status)                       \
 {                                                                            \
-    return float ## s ## _compare_internal(a, b, 1 STATUS_VAR);              \
+    return float ## s ## _compare_internal(a, b, 1, status);              \
 }

 COMPARE(32, 0xff)
====endit====

thanks
-- PMM

Re: [Qemu-devel] This patch hunk will hang scripts/checkpatch.pl...

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> The following patch file (a cut down lump of an actual change
> I was making) seems to cause scripts/checkpatch.pl to hang.
> I find that script too opaque to try to debug, but I post the
> offending input here in case anybody else wishes to investigate.

That's entertaining; observations:
   1) The kernel's checkpatch doesn't hang on it (latest 3.19-rc6)
   2) Ours hangs in the annotate_values function, repeatedly going
around:
main::annotate_values(scripts/checkpatch.pl:859):
859:                    @av_paren_type = ('E') if ($#av_paren_type < 0);
  DB<2> 
main::annotate_values(scripts/checkpatch.pl:860):
860:                    print " <" . join('', @av_paren_type) .
861:                                    "> <$type> <$av_pending>" if ($dbg_values > 1);
  DB<2> 
main::annotate_values(scripts/checkpatch.pl:862):
862:                    if ($cur =~ /^(\s+)/o) {
  DB<2> 
main::annotate_values(scripts/checkpatch.pl:879):
879:                            print "MODIFIER($1)\n" if ($dbg_values > 1);
  DB<2> 
main::annotate_values(scripts/checkpatch.pl:880):
880:                            $type = 'T';
  DB<2> 
main::annotate_values(scripts/checkpatch.pl:1040):
1040:                   if (defined $1) {
  DB<2> 
main::annotate_values(scripts/checkpatch.pl:1041):
1041:                           $cur = substr($cur, length($1));
  DB<2> 
main::annotate_values(scripts/checkpatch.pl:1042):
1042:                           $res .= $type x length($1);

$1=
$cur=float ## s ## _compare_quiet(float ## s a, float ## s b,                 \
$type=T
$res=ETTTT

    3) our annotate_values function is the same as the kernels except
       for one char difference in the 'cast' part, which doesn't fix it.

Dave

> ====begin====
> commit 4b3cf48c7fa455eb3b31dc844c959964e750a025
> Author: Peter Maydell <peter.maydell@linaro.org>
> Date:   Mon Feb 2 18:47:16 2015 +0000
> 
>     This patch will hang checkpatch.pl.
> 
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 1110a7e..ab6a93c 100644
> @@ -7218,13 +7325,13 @@ static inline int float ## s ##
> _compare_internal(float ## s a, float ## s b,\
>                                                                               \
>  int float ## s ## _compare(float ## s a, float ## s b, float_status *status) \
>  {                                                                            \
> -    return float ## s ## _compare_internal(a, b, 0 STATUS_VAR);              \
> +    return float ## s ## _compare_internal(a, b, 0, status);              \
>  }                                                                            \
>                                                                               \
>  int float ## s ## _compare_quiet(float ## s a, float ## s b,                 \
>                                   float_status *status)                       \
>  {                                                                            \
> -    return float ## s ## _compare_internal(a, b, 1 STATUS_VAR);              \
> +    return float ## s ## _compare_internal(a, b, 1, status);              \
>  }
> 
>  COMPARE(32, 0xff)
> ====endit====
> 
> thanks
> -- PMM
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Christian Borntraeger]

From: Andy Whitcroft <apw@canonical.com>

checkpatch currently loops on fpu/softfloat.c
Turns out this is fixed in the Linux version of checkpatch.

So this is a port of Andy Whitcrofts fix from Linux,
Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
valid modifier")

Cc: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 scripts/checkpatch.pl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5df61f9..8635f4c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1061,7 +1061,9 @@ sub possible {
 			case|
 			else|
 			asm|__asm__|
-			do
+			do|
+			\#|
+			\#\#|
 		)(?:\s|$)|
 		^(?:typedef|struct|enum)\b
 	    )}x;
-- 
1.9.3

Re: [Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Paolo Bonzini]

On 09/02/2015 13:18, Christian Borntraeger wrote:
> From: Andy Whitcroft <apw@canonical.com>
> 
> checkpatch currently loops on fpu/softfloat.c
> Turns out this is fixed in the Linux version of checkpatch.
> 
> So this is a port of Andy Whitcrofts fix from Linux,
> Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
> valid modifier")

I think you need to send it as a toplevel patch for Peter to pick it up.

Paolo

> Cc: Andy Whitcroft <apw@canonical.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  scripts/checkpatch.pl | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 5df61f9..8635f4c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1061,7 +1061,9 @@ sub possible {
>  			case|
>  			else|
>  			asm|__asm__|
> -			do
> +			do|
> +			\#|
> +			\#\#|
>  		)(?:\s|$)|
>  		^(?:typedef|struct|enum)\b
>  	    )}x;
> 

Re: [Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Christian Borntraeger]

Am 09.02.2015 um 15:31 schrieb Paolo Bonzini:
> 
> 
> On 09/02/2015 13:18, Christian Borntraeger wrote:
>> From: Andy Whitcroft <apw@canonical.com>
>>
>> checkpatch currently loops on fpu/softfloat.c
>> Turns out this is fixed in the Linux version of checkpatch.
>>
>> So this is a port of Andy Whitcrofts fix from Linux,
>> Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
>> valid modifier")
> 
> I think you need to send it as a toplevel patch for Peter to pick it up.

What do you mean? This patch is on top of qemu/master - if I dont messed up.

Christian
> 
> Paolo
> 
>> Cc: Andy Whitcroft <apw@canonical.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  scripts/checkpatch.pl | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 5df61f9..8635f4c 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1061,7 +1061,9 @@ sub possible {
>>  			case|
>>  			else|
>>  			asm|__asm__|
>> -			do
>> +			do|
>> +			\#|
>> +			\#\#|
>>  		)(?:\s|$)|
>>  		^(?:typedef|struct|enum)\b
>>  	    )}x;
>>
> 

Re: [Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Don Slutz]

On 02/09/15 10:08, Christian Borntraeger wrote:
> Am 09.02.2015 um 15:31 schrieb Paolo Bonzini:
>>
>>
>> On 09/02/2015 13:18, Christian Borntraeger wrote:
>>> From: Andy Whitcroft <apw@canonical.com>
>>>
>>> checkpatch currently loops on fpu/softfloat.c
>>> Turns out this is fixed in the Linux version of checkpatch.
>>>
>>> So this is a port of Andy Whitcrofts fix from Linux,
>>> Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
>>> valid modifier")
>>
>> I think you need to send it as a toplevel patch for Peter to pick it up.
> 
> What do you mean? This patch is on top of qemu/master - if I dont messed up.
> 

The patch email headers:

...
Message-ID: <1423484293-46903-1-git-send-email-borntraeger@de.ibm.com>
X-Mailer: git-send-email 1.9.3
In-Reply-To:
<CAFEAcA9dvehzeL1T+BT3N+_GJtGoL4ig_opv7rVT-0eJekLAvg@mail.gmail.com>
References:
<CAFEAcA9dvehzeL1T+BT3N+_GJtGoL4ig_opv7rVT-0eJekLAvg@mail.gmail.com>
...

Say that it is a reply, not a new email thread.

  -Don Slutz


> Christian
>>
>> Paolo
>>
>>> Cc: Andy Whitcroft <apw@canonical.com>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>  scripts/checkpatch.pl | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index 5df61f9..8635f4c 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -1061,7 +1061,9 @@ sub possible {
>>>  			case|
>>>  			else|
>>>  			asm|__asm__|
>>> -			do
>>> +			do|
>>> +			\#|
>>> +			\#\#|
>>>  		)(?:\s|$)|
>>>  		^(?:typedef|struct|enum)\b
>>>  	    )}x;
>>>
>>
> 
> 

[Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Christian Borntraeger]

From: Andy Whitcroft <apw@canonical.com>

checkpatch currently loops on fpu/softfloat.c
Turns out this is fixed in the Linux version of checkpatch.

So this is a port of Andy Whitcrofts fix from Linux,
Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
valid modifier")

Cc: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 scripts/checkpatch.pl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5df61f9..8635f4c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1061,7 +1061,9 @@ sub possible {
 			case|
 			else|
 			asm|__asm__|
-			do
+			do|
+			\#|
+			\#\#|
 		)(?:\s|$)|
 		^(?:typedef|struct|enum)\b
 	    )}x;
-- 
1.9.3

Re: [Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Peter Maydell]

On 9 February 2015 at 19:43, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
> From: Andy Whitcroft <apw@canonical.com>
>
> checkpatch currently loops on fpu/softfloat.c
> Turns out this is fixed in the Linux version of checkpatch.
>
> So this is a port of Andy Whitcrofts fix from Linux,
> Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
> valid modifier")
>
> Cc: Andy Whitcroft <apw@canonical.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  scripts/checkpatch.pl | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 5df61f9..8635f4c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1061,7 +1061,9 @@ sub possible {
>                         case|
>                         else|
>                         asm|__asm__|
> -                       do
> +                       do|
> +                       \#|
> +                       \#\#|

Are you sure this line should end with a '|', given it's the
last item in the alternation ?

>                 )(?:\s|$)|
>                 ^(?:typedef|struct|enum)\b
>             )}x;
> --
> 1.9.3
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Christian Borntraeger]

Am 09.02.2015 um 23:54 schrieb Peter Maydell:
> On 9 February 2015 at 19:43, Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
>> From: Andy Whitcroft <apw@canonical.com>
>>
>> checkpatch currently loops on fpu/softfloat.c
>> Turns out this is fixed in the Linux version of checkpatch.
>>
>> So this is a port of Andy Whitcrofts fix from Linux,
>> Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
>> valid modifier")
>>
>> Cc: Andy Whitcroft <apw@canonical.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  scripts/checkpatch.pl | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 5df61f9..8635f4c 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1061,7 +1061,9 @@ sub possible {
>>                         case|
>>                         else|
>>                         asm|__asm__|
>> -                       do
>> +                       do|
>> +                       \#|
>> +                       \#\#|
> 
> Are you sure this line should end with a '|', given it's the
> last item in the alternation ?

Good question - dont know. But the kernel version looks the same.

Re: [Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Christian Borntraeger]

Am 09.02.2015 um 23:54 schrieb Peter Maydell:
> On 9 February 2015 at 19:43, Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
>> From: Andy Whitcroft <apw@canonical.com>
>>
>> checkpatch currently loops on fpu/softfloat.c
>> Turns out this is fixed in the Linux version of checkpatch.
>>
>> So this is a port of Andy Whitcrofts fix from Linux,
>> Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
>> valid modifier")
>>
>> Cc: Andy Whitcroft <apw@canonical.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  scripts/checkpatch.pl | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 5df61f9..8635f4c 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1061,7 +1061,9 @@ sub possible {
>>                         case|
>>                         else|
>>                         asm|__asm__|
>> -                       do
>> +                       do|
>> +                       \#|
>> +                       \#\#|
> 
> Are you sure this line should end with a '|', given it's the
> last item in the alternation ?

Getting rid of that additional | also works, but both ways fix the endless loop.
Certainly not having the "|" seems cleaner. (No idea why it works).
Shall I respin without the "|"? I will then also send a patch for the kernel, in case we resync again in the future.

Christian

Re: [Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"

[Markus Armbruster]

Christian Borntraeger <borntraeger@de.ibm.com> writes:

> Am 09.02.2015 um 23:54 schrieb Peter Maydell:
>> On 9 February 2015 at 19:43, Christian Borntraeger
>> <borntraeger@de.ibm.com> wrote:
>>> From: Andy Whitcroft <apw@canonical.com>
>>>
>>> checkpatch currently loops on fpu/softfloat.c
>>> Turns out this is fixed in the Linux version of checkpatch.
>>>
>>> So this is a port of Andy Whitcrofts fix from Linux,
>>> Original commit was commit 89a883530fe7 ("checkpatch: ## is not a
>>> valid modifier")
>>>
>>> Cc: Andy Whitcroft <apw@canonical.com>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>  scripts/checkpatch.pl | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index 5df61f9..8635f4c 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -1061,7 +1061,9 @@ sub possible {
>>>                         case|
>>>                         else|
>>>                         asm|__asm__|
>>> -                       do
>>> +                       do|
>>> +                       \#|
>>> +                       \#\#|
>> 
>> Are you sure this line should end with a '|', given it's the
>> last item in the alternation ?
>
> Getting rid of that additional | also works, but both ways fix the endless loop.
> Certainly not having the "|" seems cleaner. (No idea why it works).
> Shall I respin without the "|"? I will then also send a patch for the
> kernel, in case we resync again in the future.

If it works, I'd take it as is into QEMU, and get the cleanup from the
kernel in the next resync.