[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[skunberg.kelsey]

When a range behind a bridge is empty, '[empty]' will be displayed
after the prefix. Removing option to display 'None' for consistency.

Using -vvv will include full range addresses between prefix and '[empty]'.

show_range() is only called when verbose (-v). Code checking for
'not verbose' is not needed.

Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
---
 lspci.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/lspci.c b/lspci.c
index d63b6d0..65cbde9 100644
--- a/lspci.c
+++ b/lspci.c
@@ -375,23 +375,15 @@ show_size(u64 x)
 
 static void
 show_range(char *prefix, u64 base, u64 limit, int is_64bit)
-{
-  if (base > limit)
+{ 
+  printf("%s:", prefix);
+  if (base <= limit || verbose > 2)
     {
-      if (!verbose)
-	return;
-      else if (verbose < 3)
-	{
-	  printf("%s: None\n", prefix);
-	  return;
-	}
+      if (is_64bit)
+        printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
+      else
+        printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
     }
-
-  printf("%s: ", prefix);
-  if (is_64bit)
-    printf("%016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
-  else
-    printf("%08x-%08x", (unsigned) base, (unsigned) limit);
   if (base <= limit)
     show_size(limit - base + 1);
   else
-- 
2.20.1

[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[Kelsey Skunberg]

When a range behind a bridge is empty, '[empty]' will be displayed
after the prefix. Removing option to display 'None' for consistency.

Using -vvv will include full range addresses between prefix and '[empty]'.

show_range() is only called when verbose (-v). Code checking for
'not verbose' is not needed.

Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
---
 lspci.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/lspci.c b/lspci.c
index d63b6d0..65cbde9 100644
--- a/lspci.c
+++ b/lspci.c
@@ -375,23 +375,15 @@ show_size(u64 x)
 
 static void
 show_range(char *prefix, u64 base, u64 limit, int is_64bit)
-{
-  if (base > limit)
+{ 
+  printf("%s:", prefix);
+  if (base <= limit || verbose > 2)
     {
-      if (!verbose)
-	return;
-      else if (verbose < 3)
-	{
-	  printf("%s: None\n", prefix);
-	  return;
-	}
+      if (is_64bit)
+        printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
+      else
+        printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
     }
-
-  printf("%s: ", prefix);
-  if (is_64bit)
-    printf("%016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
-  else
-    printf("%08x-%08x", (unsigned) base, (unsigned) limit);
   if (base <= limit)
     show_size(limit - base + 1);
   else
-- 
2.20.1

[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[helgaas]

Hi Kelsey,

On Tue, May 07, 2019 at 11:21:18PM -0600, Kelsey Skunberg wrote:
> When a range behind a bridge is empty, '[empty]' will be displayed
> after the prefix. Removing option to display 'None' for consistency.

1) I don't think Martin enforces this for pciutils, but for the kernel
we prefer to use imperative mood in the commit log as you did in the
subject, e.g., say "Remove" instead of "Removing".

2) The commit log doesn't quite make it clear what the purpose of this
is.  I *think* it's so that "lspci -vv" and "lspci -vvv" both use the
same text to describe a disabled window.  Saying that explicitly,
along with sample output, would help, e.g., the current output is:

  Memory behind bridge: None                          # lspci -vv
  Memory behind bridge: 00200000-001fffff [empty]     # lspci -vvv

and you're changing it so both -vv and -vvv use "[empty]":

  Memory behind bridge: [empty]                       # lspci -vv
  Memory behind bridge: 00200000-001fffff [empty]     # lspci -vvv

Personally I think that if we make them both the same, using "[None]"
instead of "[empty]" would read better, at least for -vv.

But "[empty]" does make good sense for the -vvv case, so maybe the
existing code is better even with the inconsistency.  I guess that's
up to Martin.

> Using -vvv will include full range addresses between prefix and '[empty]'.
> 
> show_range() is only called when verbose (-v). Code checking for
> 'not verbose' is not needed.

Nice simplification!  I would split this into a separate patch because
it's not logically related to changing the text and it will make both
patches easier to read.  Also, it would make it easy for Martin to
choose whether he wants to apply one, both, or neither.

If/when you repost, be sure to add "[PATCH v2]" in the subject and,
since there's more than one patch, ideally, add a "[PATCH v2 0/n]"
cover letter, with each patch as a response to the cover letter.  That
helps keep them all together.

> Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> ---
>  lspci.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)

Pay close attention to the diff.  In this case you accidentally added
a whitespace error (space at end of line).  Other ones to avoid are
space followed by tab and blank lines at end of file.  Some projects
avoid tabs completely (using spaces instead); you just have to notice
the previous practice in the file and follow that.

> diff --git a/lspci.c b/lspci.c
> index d63b6d0..65cbde9 100644
> --- a/lspci.c
> +++ b/lspci.c
> @@ -375,23 +375,15 @@ show_size(u64 x)
>  
>  static void
>  show_range(char *prefix, u64 base, u64 limit, int is_64bit)
> -{

Here's the spurious whitespace change.

> -  if (base > limit)
> +{ 
> +  printf("%s:", prefix);
> +  if (base <= limit || verbose > 2)

I like how your new code only uses "base <= limit" instead of how the
previous code used both "base <= limit" and "base > limit".  They're
basically testing for the same thing, so it was a small mental burden
to have to recognize them as related.

>      {
> -      if (!verbose)
> -	return;
> -      else if (verbose < 3)
> -	{
> -	  printf("%s: None\n", prefix);
> -	  return;
> -	}
> +      if (is_64bit)
> +        printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> +      else
> +        printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
>      }
> -
> -  printf("%s: ", prefix);
> -  if (is_64bit)
> -    printf("%016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> -  else
> -    printf("%08x-%08x", (unsigned) base, (unsigned) limit);
>    if (base <= limit)
>      show_size(limit - base + 1);
>    else
> -- 
> 2.20.1
> 

[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[Bjorn Helgaas]

Hi Kelsey,

On Tue, May 07, 2019 at 11:21:18PM -0600, Kelsey Skunberg wrote:
> When a range behind a bridge is empty, '[empty]' will be displayed
> after the prefix. Removing option to display 'None' for consistency.

1) I don't think Martin enforces this for pciutils, but for the kernel
we prefer to use imperative mood in the commit log as you did in the
subject, e.g., say "Remove" instead of "Removing".

2) The commit log doesn't quite make it clear what the purpose of this
is.  I *think* it's so that "lspci -vv" and "lspci -vvv" both use the
same text to describe a disabled window.  Saying that explicitly,
along with sample output, would help, e.g., the current output is:

  Memory behind bridge: None                          # lspci -vv
  Memory behind bridge: 00200000-001fffff [empty]     # lspci -vvv

and you're changing it so both -vv and -vvv use "[empty]":

  Memory behind bridge: [empty]                       # lspci -vv
  Memory behind bridge: 00200000-001fffff [empty]     # lspci -vvv

Personally I think that if we make them both the same, using "[None]"
instead of "[empty]" would read better, at least for -vv.

But "[empty]" does make good sense for the -vvv case, so maybe the
existing code is better even with the inconsistency.  I guess that's
up to Martin.

> Using -vvv will include full range addresses between prefix and '[empty]'.
> 
> show_range() is only called when verbose (-v). Code checking for
> 'not verbose' is not needed.

Nice simplification!  I would split this into a separate patch because
it's not logically related to changing the text and it will make both
patches easier to read.  Also, it would make it easy for Martin to
choose whether he wants to apply one, both, or neither.

If/when you repost, be sure to add "[PATCH v2]" in the subject and,
since there's more than one patch, ideally, add a "[PATCH v2 0/n]"
cover letter, with each patch as a response to the cover letter.  That
helps keep them all together.

> Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> ---
>  lspci.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)

Pay close attention to the diff.  In this case you accidentally added
a whitespace error (space at end of line).  Other ones to avoid are
space followed by tab and blank lines at end of file.  Some projects
avoid tabs completely (using spaces instead); you just have to notice
the previous practice in the file and follow that.

> diff --git a/lspci.c b/lspci.c
> index d63b6d0..65cbde9 100644
> --- a/lspci.c
> +++ b/lspci.c
> @@ -375,23 +375,15 @@ show_size(u64 x)
>  
>  static void
>  show_range(char *prefix, u64 base, u64 limit, int is_64bit)
> -{

Here's the spurious whitespace change.

> -  if (base > limit)
> +{ 
> +  printf("%s:", prefix);
> +  if (base <= limit || verbose > 2)

I like how your new code only uses "base <= limit" instead of how the
previous code used both "base <= limit" and "base > limit".  They're
basically testing for the same thing, so it was a small mental burden
to have to recognize them as related.

>      {
> -      if (!verbose)
> -	return;
> -      else if (verbose < 3)
> -	{
> -	  printf("%s: None\n", prefix);
> -	  return;
> -	}
> +      if (is_64bit)
> +        printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> +      else
> +        printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
>      }
> -
> -  printf("%s: ", prefix);
> -  if (is_64bit)
> -    printf("%016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> -  else
> -    printf("%08x-%08x", (unsigned) base, (unsigned) limit);
>    if (base <= limit)
>      show_size(limit - base + 1);
>    else
> -- 
> 2.20.1
> 

[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[skunberg.kelsey]

On Wed, May 08, 2019 at 08:40:36AM -0500, Bjorn Helgaas wrote:
Hi Bjorn, 

> Hi Kelsey,
> 
> On Tue, May 07, 2019 at 11:21:18PM -0600, Kelsey Skunberg wrote:
> > When a range behind a bridge is empty, '[empty]' will be displayed
> > after the prefix. Removing option to display 'None' for consistency.
> 
> 1) I don't think Martin enforces this for pciutils, but for the kernel
> we prefer to use imperative mood in the commit log as you did in the
> subject, e.g., say "Remove" instead of "Removing".
> 
> 2) The commit log doesn't quite make it clear what the purpose of this
> is.  I *think* it's so that "lspci -vv" and "lspci -vvv" both use the
> same text to describe a disabled window.  Saying that explicitly,
> along with sample output, would help, e.g., the current output is:
> 
>   Memory behind bridge: None                          # lspci -vv
>   Memory behind bridge: 00200000-001fffff [empty]     # lspci -vvv
> 
> and you're changing it so both -vv and -vvv use "[empty]":
> 
>   Memory behind bridge: [empty]                       # lspci -vv
>   Memory behind bridge: 00200000-001fffff [empty]     # lspci -vvv
>

These are great notes. I'll make sure to maintain the imperative mood
and provide more details in commit logs. I'll update the commit log
accordingly in V2.

> Personally I think that if we make them both the same, using "[None]"
> instead of "[empty]" would read better, at least for -vv.
> 
> But "[empty]" does make good sense for the -vvv case, so maybe the
> existing code is better even with the inconsistency.  I guess that's
> up to Martin.
>

I went through this same thought. I agree I like the 'None' for -v/-vv,
though the '[Empty]' suites better when following the range. I debated
for a while on which route to go here. 'None' didn't seem to fit after
the ranges, which is why '[Empty]' was used for all verbose options
instead. 

If you and Martin also prefer the existing output despite the inconsistency,
I can submit a patch to instead only remove the dead code (the !verbose check)
and simplify some of the code. 

> > Using -vvv will include full range addresses between prefix and '[empty]'.
> > 
> > show_range() is only called when verbose (-v). Code checking for
> > 'not verbose' is not needed.
> 
> Nice simplification!  I would split this into a separate patch because
> it's not logically related to changing the text and it will make both
> patches easier to read.  Also, it would make it easy for Martin to
> choose whether he wants to apply one, both, or neither.
>

In the event the two patches would cause a merge conflict if both
applied, is it still proper to submit them both? I can place the
'!verbose' check back into this patch, though there would still be
some changes to make it fit the new structure. 

> If/when you repost, be sure to add "[PATCH v2]" in the subject and,
> since there's more than one patch, ideally, add a "[PATCH v2 0/n]"
> cover letter, with each patch as a response to the cover letter.  That
> helps keep them all together.
> 
> > Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> > ---
> >  lspci.c | 22 +++++++---------------
> >  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> Pay close attention to the diff.  In this case you accidentally added
> a whitespace error (space at end of line).  Other ones to avoid are
> space followed by tab and blank lines at end of file.  Some projects
> avoid tabs completely (using spaces instead); you just have to notice
> the previous practice in the file and follow that.
>

Ah! I was so focused on the leading whitespace I didn't check the trailing!
I'll keep closer attention to this. Thank you! 

> > diff --git a/lspci.c b/lspci.c
> > index d63b6d0..65cbde9 100644
> > --- a/lspci.c
> > +++ b/lspci.c
> > @@ -375,23 +375,15 @@ show_size(u64 x)
> >  
> >  static void
> >  show_range(char *prefix, u64 base, u64 limit, int is_64bit)
> > -{
> 
> Here's the spurious whitespace change.
> 
> > -  if (base > limit)
> > +{ 
> > +  printf("%s:", prefix);
> > +  if (base <= limit || verbose > 2)
> 
> I like how your new code only uses "base <= limit" instead of how the
> previous code used both "base <= limit" and "base > limit".  They're
> basically testing for the same thing, so it was a small mental burden
> to have to recognize them as related.
> 
> >      {
> > -      if (!verbose)
> > -	return;
> > -      else if (verbose < 3)
> > -	{
> > -	  printf("%s: None\n", prefix);
> > -	  return;
> > -	}
> > +      if (is_64bit)
> > +        printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> > +      else
> > +        printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
> >      }
> > -
> > -  printf("%s: ", prefix);
> > -  if (is_64bit)
> > -    printf("%016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> > -  else
> > -    printf("%08x-%08x", (unsigned) base, (unsigned) limit);
> >    if (base <= limit)
> >      show_size(limit - base + 1);
> >    else
> > -- 
> > 2.20.1
> > 

Thank you for the in-depth feedback! I appreciate it a lot! 

-Kelsey 

[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[Kelsey Skunberg]

On Wed, May 08, 2019 at 08:40:36AM -0500, Bjorn Helgaas wrote:
Hi Bjorn, 

> Hi Kelsey,
> 
> On Tue, May 07, 2019 at 11:21:18PM -0600, Kelsey Skunberg wrote:
> > When a range behind a bridge is empty, '[empty]' will be displayed
> > after the prefix. Removing option to display 'None' for consistency.
> 
> 1) I don't think Martin enforces this for pciutils, but for the kernel
> we prefer to use imperative mood in the commit log as you did in the
> subject, e.g., say "Remove" instead of "Removing".
> 
> 2) The commit log doesn't quite make it clear what the purpose of this
> is.  I *think* it's so that "lspci -vv" and "lspci -vvv" both use the
> same text to describe a disabled window.  Saying that explicitly,
> along with sample output, would help, e.g., the current output is:
> 
>   Memory behind bridge: None                          # lspci -vv
>   Memory behind bridge: 00200000-001fffff [empty]     # lspci -vvv
> 
> and you're changing it so both -vv and -vvv use "[empty]":
> 
>   Memory behind bridge: [empty]                       # lspci -vv
>   Memory behind bridge: 00200000-001fffff [empty]     # lspci -vvv
>

These are great notes. I'll make sure to maintain the imperative mood
and provide more details in commit logs. I'll update the commit log
accordingly in V2.

> Personally I think that if we make them both the same, using "[None]"
> instead of "[empty]" would read better, at least for -vv.
> 
> But "[empty]" does make good sense for the -vvv case, so maybe the
> existing code is better even with the inconsistency.  I guess that's
> up to Martin.
>

I went through this same thought. I agree I like the 'None' for -v/-vv,
though the '[Empty]' suites better when following the range. I debated
for a while on which route to go here. 'None' didn't seem to fit after
the ranges, which is why '[Empty]' was used for all verbose options
instead. 

If you and Martin also prefer the existing output despite the inconsistency,
I can submit a patch to instead only remove the dead code (the !verbose check)
and simplify some of the code. 

> > Using -vvv will include full range addresses between prefix and '[empty]'.
> > 
> > show_range() is only called when verbose (-v). Code checking for
> > 'not verbose' is not needed.
> 
> Nice simplification!  I would split this into a separate patch because
> it's not logically related to changing the text and it will make both
> patches easier to read.  Also, it would make it easy for Martin to
> choose whether he wants to apply one, both, or neither.
>

In the event the two patches would cause a merge conflict if both
applied, is it still proper to submit them both? I can place the
'!verbose' check back into this patch, though there would still be
some changes to make it fit the new structure. 

> If/when you repost, be sure to add "[PATCH v2]" in the subject and,
> since there's more than one patch, ideally, add a "[PATCH v2 0/n]"
> cover letter, with each patch as a response to the cover letter.  That
> helps keep them all together.
> 
> > Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> > ---
> >  lspci.c | 22 +++++++---------------
> >  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> Pay close attention to the diff.  In this case you accidentally added
> a whitespace error (space at end of line).  Other ones to avoid are
> space followed by tab and blank lines at end of file.  Some projects
> avoid tabs completely (using spaces instead); you just have to notice
> the previous practice in the file and follow that.
>

Ah! I was so focused on the leading whitespace I didn't check the trailing!
I'll keep closer attention to this. Thank you! 

> > diff --git a/lspci.c b/lspci.c
> > index d63b6d0..65cbde9 100644
> > --- a/lspci.c
> > +++ b/lspci.c
> > @@ -375,23 +375,15 @@ show_size(u64 x)
> >  
> >  static void
> >  show_range(char *prefix, u64 base, u64 limit, int is_64bit)
> > -{
> 
> Here's the spurious whitespace change.
> 
> > -  if (base > limit)
> > +{ 
> > +  printf("%s:", prefix);
> > +  if (base <= limit || verbose > 2)
> 
> I like how your new code only uses "base <= limit" instead of how the
> previous code used both "base <= limit" and "base > limit".  They're
> basically testing for the same thing, so it was a small mental burden
> to have to recognize them as related.
> 
> >      {
> > -      if (!verbose)
> > -	return;
> > -      else if (verbose < 3)
> > -	{
> > -	  printf("%s: None\n", prefix);
> > -	  return;
> > -	}
> > +      if (is_64bit)
> > +        printf(" %016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> > +      else
> > +        printf(" %08x-%08x", (unsigned) base, (unsigned) limit);
> >      }
> > -
> > -  printf("%s: ", prefix);
> > -  if (is_64bit)
> > -    printf("%016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
> > -  else
> > -    printf("%08x-%08x", (unsigned) base, (unsigned) limit);
> >    if (base <= limit)
> >      show_size(limit - base + 1);
> >    else
> > -- 
> > 2.20.1
> > 

Thank you for the in-depth feedback! I appreciate it a lot! 

-Kelsey 

[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[helgaas]

On Wed, May 08, 2019 at 11:35:41PM -0600, Kelsey Skunberg wrote:
> On Wed, May 08, 2019 at 08:40:36AM -0500, Bjorn Helgaas wrote:
> > On Tue, May 07, 2019 at 11:21:18PM -0600, Kelsey Skunberg wrote:

> > > show_range() is only called when verbose (-v). Code checking for
> > > 'not verbose' is not needed.
> > 
> > Nice simplification!  I would split this into a separate patch because
> > it's not logically related to changing the text and it will make both
> > patches easier to read.  Also, it would make it easy for Martin to
> > choose whether he wants to apply one, both, or neither.
> 
> In the event the two patches would cause a merge conflict if both
> applied, is it still proper to submit them both? I can place the
> '!verbose' check back into this patch, though there would still be
> some changes to make it fit the new structure. 

The two patches would constitute a "series".  If applied in order they
should not cause conflicts.  What I would do in this situation is make
the first one do the cleanup because that should be uncontroversial
(if it's correct and causes no user-visible change) and make the
second do the wording change.  Then Martin could easily apply the
first and ignore the second.

> Ah! I was so focused on the leading whitespace I didn't check the trailing!
> I'll keep closer attention to this. Thank you! 

Many editors can highlight whitespace errors for you.  I have the
following in my .vimrc:

  " Show trailing whitespace and spaces before tabs
  hi link localWhitespaceError Error
  au Syntax * syn match localWhitespaceError /\(\zs\%#\|\s\)\+$/ display
  au Syntax * syn match localWhitespaceError / \+\ze\t/ display

But in general you should look through the diff and make sure it
includes only the changes you intend.

Bjorn

[Linux-kernel-mentees] [PATCH] lspci: Make output for empty range behind a bridge consistent

[Bjorn Helgaas]

On Wed, May 08, 2019 at 11:35:41PM -0600, Kelsey Skunberg wrote:
> On Wed, May 08, 2019 at 08:40:36AM -0500, Bjorn Helgaas wrote:
> > On Tue, May 07, 2019 at 11:21:18PM -0600, Kelsey Skunberg wrote:

> > > show_range() is only called when verbose (-v). Code checking for
> > > 'not verbose' is not needed.
> > 
> > Nice simplification!  I would split this into a separate patch because
> > it's not logically related to changing the text and it will make both
> > patches easier to read.  Also, it would make it easy for Martin to
> > choose whether he wants to apply one, both, or neither.
> 
> In the event the two patches would cause a merge conflict if both
> applied, is it still proper to submit them both? I can place the
> '!verbose' check back into this patch, though there would still be
> some changes to make it fit the new structure. 

The two patches would constitute a "series".  If applied in order they
should not cause conflicts.  What I would do in this situation is make
the first one do the cleanup because that should be uncontroversial
(if it's correct and causes no user-visible change) and make the
second do the wording change.  Then Martin could easily apply the
first and ignore the second.

> Ah! I was so focused on the leading whitespace I didn't check the trailing!
> I'll keep closer attention to this. Thank you! 

Many editors can highlight whitespace errors for you.  I have the
following in my .vimrc:

  " Show trailing whitespace and spaces before tabs
  hi link localWhitespaceError Error
  au Syntax * syn match localWhitespaceError /\(\zs\%#\|\s\)\+$/ display
  au Syntax * syn match localWhitespaceError / \+\ze\t/ display

But in general you should look through the diff and make sure it
includes only the changes you intend.

Bjorn