checkpatch: reversed logic with acpi test checks

[PATCH] checkpatch: reversed logic with acpi test checks

Posted by Michael S. Tsirkin 5 years, 8 months ago

Logic reversed: allowed list should just be ignored. Instead we
only take that into account :(

Fixes: e11b06a880ca ("checkpatch: ignore allowed diff list")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0ba213e9f2..55aa45dc16 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1267,7 +1267,7 @@ sub checkfilename {
         # files and when changing tests.
 	if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) {
 		$$acpi_testexpected = $name;
-	} elsif ($name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
+	} elsif (not $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
 		$$acpi_nontestexpected = $name;
 	}
 	if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
-- 
MST

Re: [PATCH] checkpatch: reversed logic with acpi test checks

Posted by Philippe Mathieu-Daudé 5 years, 8 months ago

On 6/2/20 7:36 AM, Michael S. Tsirkin wrote:
> Logic reversed: allowed list should just be ignored. Instead we
> only take that into account :(
> 
> Fixes: e11b06a880ca ("checkpatch: ignore allowed diff list")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  scripts/checkpatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 0ba213e9f2..55aa45dc16 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1267,7 +1267,7 @@ sub checkfilename {
>          # files and when changing tests.
>  	if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) {
>  		$$acpi_testexpected = $name;
> -	} elsif ($name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> +	} elsif (not $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
>  		$$acpi_nontestexpected = $name;
>  	}
>  	if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH] checkpatch: reversed logic with acpi test checks

Posted by Auger Eric 5 years, 8 months ago

Hi,

On 6/2/20 7:36 AM, Michael S. Tsirkin wrote:
> Logic reversed: allowed list should just be ignored. Instead we
> only take that into account :(
> 
> Fixes: e11b06a880ca ("checkpatch: ignore allowed diff list")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Fixes issues reported in "[RFC 0/6] TPM-TIS bios-tables-test" cover letter

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric



> ---
>  scripts/checkpatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 0ba213e9f2..55aa45dc16 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1267,7 +1267,7 @@ sub checkfilename {
>          # files and when changing tests.
>  	if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) {
>  		$$acpi_testexpected = $name;
> -	} elsif ($name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> +	} elsif (not $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
>  		$$acpi_nontestexpected = $name;
>  	}
>  	if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
>

Re: [PATCH] checkpatch: reversed logic with acpi test checks

Posted by Paolo Bonzini 5 years, 8 months ago

On 02/06/20 07:36, Michael S. Tsirkin wrote:
> Logic reversed: allowed list should just be ignored. Instead we
> only take that into account :(
> 
> Fixes: e11b06a880ca ("checkpatch: ignore allowed diff list")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  scripts/checkpatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 0ba213e9f2..55aa45dc16 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1267,7 +1267,7 @@ sub checkfilename {
>          # files and when changing tests.
>  	if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) {
>  		$$acpi_testexpected = $name;
> -	} elsif ($name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> +	} elsif (not $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
>  		$$acpi_nontestexpected = $name;
>  	}
>  	if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
> 

Queued with "!~" to achieve the logical not.

Paolo

Re: [PATCH] checkpatch: reversed logic with acpi test checks

Posted by Michael S. Tsirkin 5 years, 8 months ago

On Thu, Jun 04, 2020 at 08:45:15PM +0200, Paolo Bonzini wrote:
> On 02/06/20 07:36, Michael S. Tsirkin wrote:
> > Logic reversed: allowed list should just be ignored. Instead we
> > only take that into account :(
> > 
> > Fixes: e11b06a880ca ("checkpatch: ignore allowed diff list")
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  scripts/checkpatch.pl | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 0ba213e9f2..55aa45dc16 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -1267,7 +1267,7 @@ sub checkfilename {
> >          # files and when changing tests.
> >  	if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) {
> >  		$$acpi_testexpected = $name;
> > -	} elsif ($name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> > +	} elsif (not $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> >  		$$acpi_nontestexpected = $name;
> >  	}
> >  	if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
> > 
> 
> Queued with "!~" to achieve the logical not.
> 
> Paolo

Hmm perl manual says "not" is the logical not. Weird.
What's !~? My perl is a bit rusty :)

Re: [PATCH] checkpatch: reversed logic with acpi test checks

Posted by Peter Maydell 5 years, 8 months ago

On Thu, 4 Jun 2020 at 20:09, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Jun 04, 2020 at 08:45:15PM +0200, Paolo Bonzini wrote:
> > On 02/06/20 07:36, Michael S. Tsirkin wrote:

> > > +   } elsif (not $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> > >             $$acpi_nontestexpected = $name;
> > >     }
> > >     if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
> > >
> >
> > Queued with "!~" to achieve the logical not.
> >
> > Paolo
>
> Hmm perl manual says "not" is the logical not. Weird.
> What's !~? My perl is a bit rusty :)

man perlop says:
# Binary "!~" is just like "=~" except the return value is negated in
the logical sense.

So it's more idiomatic to use it directly rather than using =~ and then
doing a logical negation separately.

thanks
-- PMM