[PATCH v4 0/3] lspci: Update verbose help and show_range()

[PATCH v4 0/3] lspci: Update verbose help and show_range()

[Kelsey Skunberg]

Changes since v1:
  - Expand changes into more patches for easier review.
  - Combine three patches for lspci.c into patchset.

  * Patch 1: lspci: Include -vvv option in help
    No changes made. Added to series for review.

  * Patch 2: lspci: Remove unnecessary !verbose check in show_range()
    Move into it's own patch since v1. Dead code which checks for
    verbosity to be true.

  * Patch 3: lspci: Replace output for bridge with empty range from
    "None" to "[empty]"
    Patch builds off Patch 2 to change show_range() output to be more
    consistent between each level of verbosity.

Changes since v2:
  * Patch 1: Update commit log to imperative mood

  * Patch 2: Fix logical error
        Previous:
        (base > limit || verbose < 3)
        New:
        (base > limit && verbose < 3)

  * Patch 3: Fix logical error
        Previous:
        base > limit
        New:
        base <= limit

Changes since v3:
  * Patch 1 and 2: No change
  * Patch 3: Change output from "[empty]" to "[disabled]"

Kelsey Skunberg (3):
  lspci: Include -vvv option in help
  lspci: Remove unnecessary !verbose check in show_range()
  lspci: Change output for bridge with empty range to "[disabled]"

 lspci.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

--
2.20.1

[PATCH v4 1/3] lspci: Include -vvv option in help

[Kelsey Skunberg]

Include -vvv in help message.

Signed-off-by: Kelsey Skunberg <skunberg.kelsey@gmail.com>
---
 lspci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lspci.c b/lspci.c
index d63b6d0..937c6e4 100644
--- a/lspci.c
+++ b/lspci.c
@@ -41,7 +41,7 @@ static char help_msg[] =
 "-t\t\tShow bus tree\n"
 "\n"
 "Display options:\n"
-"-v\t\tBe verbose (-vv for very verbose)\n"
+"-v\t\tBe verbose (-vv or -vvv for higher verbosity)\n"
 #ifdef PCI_OS_LINUX
 "-k\t\tShow kernel drivers handling each device\n"
 #endif
-- 
2.20.1

[PATCH v4 2/3] lspci: Remove unnecessary !verbose check in show_range()

[Kelsey Skunberg]

Remove 'if (!verbose)' code in show_range() due to not being called.
show_range() will only be called when verbose is true. Additional call
to check for verbosity within show_range() is dead code.

!verbose was used so nothing would print if the range behind a bridge
had a base > limit and verbose == false. Since show_range() will not be
called when verbose == false, not printing bridge information is
still accomplished.

Signed-off-by: Kelsey Skunberg <skunberg.kelsey@gmail.com>
---
 lspci.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/lspci.c b/lspci.c
index 937c6e4..7418b07 100644
--- a/lspci.c
+++ b/lspci.c
@@ -376,17 +376,11 @@ show_size(u64 x)
 static void
 show_range(char *prefix, u64 base, u64 limit, int is_64bit)
 {
-  if (base > limit)
+  if (base > limit && verbose < 3)
     {
-      if (!verbose)
-	return;
-      else if (verbose < 3)
-	{
-	  printf("%s: None\n", prefix);
-	  return;
-	}
+      printf("%s: None\n", prefix);
+      return;
     }
-
   printf("%s: ", prefix);
   if (is_64bit)
     printf("%016" PCI_U64_FMT_X "-%016" PCI_U64_FMT_X, base, limit);
-- 
2.20.1

[PATCH v4 3/3] lspci: Change output for bridge with empty range to "[disabled]"

[Kelsey Skunberg]

Change output displayed for memory behind bridge when the range is
empty to be consistent between each verbosity level. Replace "None" and
"[empty]" with "[disabled]". Old and new output examples listed below
for each verbosity level.

Show_range() is not called unless verbose == true. No output given
unless a verbose argument is provided.

OLD output for -v and -vv which uses "None" and -vvv uses "[empty]":

  Memory behind bridge: None                          # lspci -v
  Memory behind bridge: None                          # lspci -vv
  Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv

NEW output for -v, -vv, and -vvv to use "[disabled]":

  Memory behind bridge: [disabled]                       # lspci -v
  Memory behind bridge: [disabled]                       # lspci -vv
  Memory behind bridge: 0000e000-0000efff [disabled]     # lspci -vvv

Advantage is consistent output regardless of verbosity level chosen and
to simplify the code.

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

diff --git a/lspci.c b/lspci.c
index 7418b07..525bb82 100644
--- a/lspci.c
+++ b/lspci.c
@@ -376,20 +376,18 @@ show_size(u64 x)
 static void
 show_range(char *prefix, u64 base, u64 limit, int is_64bit)
 {
-  if (base > limit && verbose < 3)
+  printf("%s:", prefix);
+  if (base <= limit || verbose > 2)
     {
-      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
-    printf(" [empty]");
+    printf(" [disabled]");
   putchar('\n');
 }
 
-- 
2.20.1

Re: [PATCH v4 0/3] lspci: Update verbose help and show_range()

[Bjorn Helgaas]

On Wed, Jun 19, 2019 at 10:48:55AM -0600, Kelsey Skunberg wrote:
> Changes since v1:
>   - Expand changes into more patches for easier review.
>   - Combine three patches for lspci.c into patchset.
> 
>   * Patch 1: lspci: Include -vvv option in help
>     No changes made. Added to series for review.
> 
>   * Patch 2: lspci: Remove unnecessary !verbose check in show_range()
>     Move into it's own patch since v1. Dead code which checks for
>     verbosity to be true.
> 
>   * Patch 3: lspci: Replace output for bridge with empty range from
>     "None" to "[empty]"
>     Patch builds off Patch 2 to change show_range() output to be more
>     consistent between each level of verbosity.
> 
> Changes since v2:
>   * Patch 1: Update commit log to imperative mood
> 
>   * Patch 2: Fix logical error
>         Previous:
>         (base > limit || verbose < 3)
>         New:
>         (base > limit && verbose < 3)
> 
>   * Patch 3: Fix logical error
>         Previous:
>         base > limit
>         New:
>         base <= limit
> 
> Changes since v3:
>   * Patch 1 and 2: No change
>   * Patch 3: Change output from "[empty]" to "[disabled]"
> 
> Kelsey Skunberg (3):
>   lspci: Include -vvv option in help
>   lspci: Remove unnecessary !verbose check in show_range()
>   lspci: Change output for bridge with empty range to "[disabled]"
> 
>  lspci.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)

These look great to me, thanks for doing this.  FWIW:

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>

Re: [PATCH v4 0/3] lspci: Update verbose help and show_range()

[Martin Mareš]

Hello!

> Kelsey Skunberg (3):
>   lspci: Include -vvv option in help
>   lspci: Remove unnecessary !verbose check in show_range()
>   lspci: Change output for bridge with empty range to "[disabled]"
> 
>  lspci.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)

Thanks, applied.  And sorry for the delay.

-- 
Martin `MJ' Mareš                        <mj@ucw.cz>   http://mj.ucw.cz/
United Computer Wizards, Prague, Czech Republic, Europe, Earth, Universe
A computer without Windows is like a chocolate cake without mustard.