[Linux-kernel-mentees] [PATCH v2 0/3] lspci: Update verbose help and show_range()

[Linux-kernel-mentees] [PATCH v2 0/3] lspci: Update verbose help and show_range()

[skunberg.kelsey]

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.

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

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

--
2.20.1

[Linux-kernel-mentees] [PATCH v2 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.

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

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

--
2.20.1

[Linux-kernel-mentees] [PATCH v2 1/3] lspci: Include -vvv option in help

[skunberg.kelsey]

Listed -vvv within the help page so highest used level of verbose
in lspci is known

Signed-off-by: Kelsey Skunberg <skunberg.kelsey at 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

[Linux-kernel-mentees] [PATCH v2 1/3] lspci: Include -vvv option in help

[Kelsey Skunberg]

Listed -vvv within the help page so highest used level of verbose
in lspci is known

Signed-off-by: Kelsey Skunberg <skunberg.kelsey at 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

[Linux-kernel-mentees] [PATCH v2 2/3] lspci: Remove unnecessary !verbose check in show_range()

[skunberg.kelsey]

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 at gmail.com>
---
 lspci.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/lspci.c b/lspci.c
index 937c6e4..419f386 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

[Linux-kernel-mentees] [PATCH v2 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 at gmail.com>
---
 lspci.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/lspci.c b/lspci.c
index 937c6e4..419f386 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

[Linux-kernel-mentees] [PATCH v2 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[skunberg.kelsey]

Change output displayed for memory behind bridge when the range is
empty to be consistent between each verbosity level. Replace 'None' with
'[empty]'. 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 ouptut 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 and -vv to also use "[empty]":

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

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

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

diff --git a/lspci.c b/lspci.c
index 419f386..c70ef61 100644
--- a/lspci.c
+++ b/lspci.c
@@ -376,17 +376,15 @@ 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)
+  if (base > limit)
     show_size(limit - base + 1);
   else
     printf(" [empty]");
-- 
2.20.1

[Linux-kernel-mentees] [PATCH v2 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[Kelsey Skunberg]

Change output displayed for memory behind bridge when the range is
empty to be consistent between each verbosity level. Replace 'None' with
'[empty]'. 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 ouptut 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 and -vv to also use "[empty]":

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

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

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

diff --git a/lspci.c b/lspci.c
index 419f386..c70ef61 100644
--- a/lspci.c
+++ b/lspci.c
@@ -376,17 +376,15 @@ 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)
+  if (base > limit)
     show_size(limit - base + 1);
   else
     printf(" [empty]");
-- 
2.20.1

[Linux-kernel-mentees] [PATCH v2 1/3] lspci: Include -vvv option in help

[bjorn.helgaas]

On Sat, May 11, 2019 at 6:05 PM Kelsey Skunberg
<skunberg.kelsey at gmail.com> wrote:
>
> Listed -vvv within the help page so highest used level of verbose
> in lspci is known

This tells what *you* did; using the imperative mood would put the
focus on what the *patch* does.  Also missing the period.  Maybe:

  Include "-vvv" in help message.

> Signed-off-by: Kelsey Skunberg <skunberg.kelsey at 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
>

[Linux-kernel-mentees] [PATCH v2 1/3] lspci: Include -vvv option in help

[Bjorn Helgaas]

On Sat, May 11, 2019 at 6:05 PM Kelsey Skunberg
<skunberg.kelsey at gmail.com> wrote:
>
> Listed -vvv within the help page so highest used level of verbose
> in lspci is known

This tells what *you* did; using the imperative mood would put the
focus on what the *patch* does.  Also missing the period.  Maybe:

  Include "-vvv" in help message.

> Signed-off-by: Kelsey Skunberg <skunberg.kelsey at 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
>

[Linux-kernel-mentees] [PATCH v2 2/3] lspci: Remove unnecessary !verbose check in show_range()

[bjorn.helgaas]

On Sat, May 11, 2019 at 6:05 PM Kelsey Skunberg
<skunberg.kelsey at gmail.com> wrote:
>
> 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 at gmail.com>
> ---
>  lspci.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/lspci.c b/lspci.c
> index 937c6e4..419f386 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;
> -       }

I don't think this works quite right.  The resulting code is:

  if (base > limit || verbose < 3) {
    printf("%s: None\n", prefix);
    return;
  }

but that means we print "None" when the window is *enabled* (base <=
limit) and verbose < 3.  We should print the window in that case.

> +      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
>

[Linux-kernel-mentees] [PATCH v2 2/3] lspci: Remove unnecessary !verbose check in show_range()

[Bjorn Helgaas]

On Sat, May 11, 2019 at 6:05 PM Kelsey Skunberg
<skunberg.kelsey at gmail.com> wrote:
>
> 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 at gmail.com>
> ---
>  lspci.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/lspci.c b/lspci.c
> index 937c6e4..419f386 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;
> -       }

I don't think this works quite right.  The resulting code is:

  if (base > limit || verbose < 3) {
    printf("%s: None\n", prefix);
    return;
  }

but that means we print "None" when the window is *enabled* (base <=
limit) and verbose < 3.  We should print the window in that case.

> +      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
>

[Linux-kernel-mentees] [PATCH v3 0/3] Update verbose help and show_range()

[skunberg.kelsey]

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

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

 lspci.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

--
2.20.1

[Linux-kernel-mentees] [PATCH v3 0/3] 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

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

 lspci.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

--
2.20.1

[Linux-kernel-mentees] [PATCH v3 1/3] lspci: Include -vvv option in help

[skunberg.kelsey]

Include -vvv in help message.

Signed-off-by: Kelsey Skunberg <skunberg.kelsey at 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

[Linux-kernel-mentees] [PATCH v3 1/3] lspci: Include -vvv option in help

[Kelsey Skunberg]

Include -vvv in help message.

Signed-off-by: Kelsey Skunberg <skunberg.kelsey at 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

[Linux-kernel-mentees] [PATCH v3 2/3] lspci: Remove unnecessary !verbose check in show_range()

[skunberg.kelsey]

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 at 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

[Linux-kernel-mentees] [PATCH v3 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 at 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

[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[skunberg.kelsey]

Change output displayed for memory behind bridge when the range is
empty to be consistent between each verbosity level. Replace 'None' with
'[empty]'. 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 ouptut 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 and -vv to also use "[empty]":

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

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

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

diff --git a/lspci.c b/lspci.c
index 7418b07..0c00c91 100644
--- a/lspci.c
+++ b/lspci.c
@@ -376,16 +376,14 @@ 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
-- 
2.20.1

[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[Kelsey Skunberg]

Change output displayed for memory behind bridge when the range is
empty to be consistent between each verbosity level. Replace 'None' with
'[empty]'. 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 ouptut 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 and -vv to also use "[empty]":

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

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

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

diff --git a/lspci.c b/lspci.c
index 7418b07..0c00c91 100644
--- a/lspci.c
+++ b/lspci.c
@@ -376,16 +376,14 @@ 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
-- 
2.20.1

[Linux-kernel-mentees] [PATCH v2 2/3] lspci: Remove unnecessary !verbose check in show_range()

[skunberg.kelsey]

On Thu, May 16, 2019 at 07:58:37AM -0500, Bjorn Helgaas wrote:
> On Sat, May 11, 2019 at 6:05 PM Kelsey Skunberg
> <skunberg.kelsey at gmail.com> wrote:
> >
> > 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 at gmail.com>
> > ---
> >  lspci.c | 12 +++---------
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/lspci.c b/lspci.c
> > index 937c6e4..419f386 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;
> > -       }
> 
> I don't think this works quite right.  The resulting code is:
> 
>   if (base > limit || verbose < 3) {
>     printf("%s: None\n", prefix);
>     return;
>   }
> 
> but that means we print "None" when the window is *enabled* (base <=
> limit) and verbose < 3.  We should print the window in that case.
> 
> > +      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
> >

You're absolutely right and I should have caught this. Revised and v3
sumbitted. Thank you for reviewing and letting me know.

[Linux-kernel-mentees] [PATCH v2 2/3] lspci: Remove unnecessary !verbose check in show_range()

[Kelsey Skunberg]

On Thu, May 16, 2019 at 07:58:37AM -0500, Bjorn Helgaas wrote:
> On Sat, May 11, 2019 at 6:05 PM Kelsey Skunberg
> <skunberg.kelsey at gmail.com> wrote:
> >
> > 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 at gmail.com>
> > ---
> >  lspci.c | 12 +++---------
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/lspci.c b/lspci.c
> > index 937c6e4..419f386 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;
> > -       }
> 
> I don't think this works quite right.  The resulting code is:
> 
>   if (base > limit || verbose < 3) {
>     printf("%s: None\n", prefix);
>     return;
>   }
> 
> but that means we print "None" when the window is *enabled* (base <=
> limit) and verbose < 3.  We should print the window in that case.
> 
> > +      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
> >

You're absolutely right and I should have caught this. Revised and v3
sumbitted. Thank you for reviewing and letting me know.

[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[bjorn.helgaas]

On Thu, May 16, 2019 at 4:23 PM Kelsey Skunberg
<skunberg.kelsey at gmail.com> wrote:
>
> Change output displayed for memory behind bridge when the range is
> empty to be consistent between each verbosity level. Replace 'None' with
> '[empty]'. 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 ouptut for -v and -vv which uses 'None' and -vvv uses '[empty]':

s/ouptut/output/

>   Memory behind bridge: None                          # lspci -v
>   Memory behind bridge: None                          # lspci -vv
>   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
>
> NEW output for -v and -vv to also use "[empty]":
>
>   Memory behind bridge: [empty]                       # lspci -v
>   Memory behind bridge: [empty]                       # lspci -vv
>   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv

How about this alternative?  The spec doesn't actually use the term
"window", but I think in terms of bridge windows to downstream
devices, and the windows can be either enabled or disabled.

  Memory behind bridge: Disabled    # lspci -v or lspci -vv
  Memory behind bridge: Disabled [0x0000e000-0x0000efff]   # lspci -vvv



> Advantage is consistent output regardless of verbosity level chosen and
> to simplify the code.
>
> Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> ---
>  lspci.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/lspci.c b/lspci.c
> index 7418b07..0c00c91 100644
> --- a/lspci.c
> +++ b/lspci.c
> @@ -376,16 +376,14 @@ 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
> --
> 2.20.1
>

[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[Bjorn Helgaas]

On Thu, May 16, 2019 at 4:23 PM Kelsey Skunberg
<skunberg.kelsey at gmail.com> wrote:
>
> Change output displayed for memory behind bridge when the range is
> empty to be consistent between each verbosity level. Replace 'None' with
> '[empty]'. 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 ouptut for -v and -vv which uses 'None' and -vvv uses '[empty]':

s/ouptut/output/

>   Memory behind bridge: None                          # lspci -v
>   Memory behind bridge: None                          # lspci -vv
>   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
>
> NEW output for -v and -vv to also use "[empty]":
>
>   Memory behind bridge: [empty]                       # lspci -v
>   Memory behind bridge: [empty]                       # lspci -vv
>   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv

How about this alternative?  The spec doesn't actually use the term
"window", but I think in terms of bridge windows to downstream
devices, and the windows can be either enabled or disabled.

  Memory behind bridge: Disabled    # lspci -v or lspci -vv
  Memory behind bridge: Disabled [0x0000e000-0x0000efff]   # lspci -vvv



> Advantage is consistent output regardless of verbosity level chosen and
> to simplify the code.
>
> Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> ---
>  lspci.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/lspci.c b/lspci.c
> index 7418b07..0c00c91 100644
> --- a/lspci.c
> +++ b/lspci.c
> @@ -376,16 +376,14 @@ 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
> --
> 2.20.1
>

[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[skunberg.kelsey]

On Tue, Jun 11, 2019 at 03:25:00PM -0500, Bjorn Helgaas wrote:
> On Thu, May 16, 2019 at 4:23 PM Kelsey Skunberg
> <skunberg.kelsey at gmail.com> wrote:
> >
> > Change output displayed for memory behind bridge when the range is
> > empty to be consistent between each verbosity level. Replace 'None' with
> > '[empty]'. 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 ouptut for -v and -vv which uses 'None' and -vvv uses '[empty]':
> 
> s/ouptut/output/
> 
> >   Memory behind bridge: None                          # lspci -v
> >   Memory behind bridge: None                          # lspci -vv
> >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> >
> > NEW output for -v and -vv to also use "[empty]":
> >
> >   Memory behind bridge: [empty]                       # lspci -v
> >   Memory behind bridge: [empty]                       # lspci -vv
> >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> 
> How about this alternative?  The spec doesn't actually use the term
> "window", but I think in terms of bridge windows to downstream
> devices, and the windows can be either enabled or disabled.
> 
>   Memory behind bridge: Disabled    # lspci -v or lspci -vv
>   Memory behind bridge: Disabled [0x0000e000-0x0000efff]   # lspci -vvv
> 

I like the alternative of using "Disabled". Could it be more suiting to use
"[disabled]" which is also used in show_bases(), show_rom(), and show_htype2()?
Then keep the range format the same. For example:

   Memory behind bridge: [disabled]    # lspci -v or lspci -vv
   Memory behind bridge: 0x0000e000-0x0000efff [disabled]   # lspci -vvv

I would be happy to submit an updated patch for either version thought to read
better.

> > Advantage is consistent output regardless of verbosity level chosen and
> > to simplify the code.
> >
> > Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> > ---
> >  lspci.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/lspci.c b/lspci.c
> > index 7418b07..0c00c91 100644
> > --- a/lspci.c
> > +++ b/lspci.c
> > @@ -376,16 +376,14 @@ 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
> > --
> > 2.20.1
> >

[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[Kelsey Skunberg]

On Tue, Jun 11, 2019 at 03:25:00PM -0500, Bjorn Helgaas wrote:
> On Thu, May 16, 2019 at 4:23 PM Kelsey Skunberg
> <skunberg.kelsey at gmail.com> wrote:
> >
> > Change output displayed for memory behind bridge when the range is
> > empty to be consistent between each verbosity level. Replace 'None' with
> > '[empty]'. 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 ouptut for -v and -vv which uses 'None' and -vvv uses '[empty]':
> 
> s/ouptut/output/
> 
> >   Memory behind bridge: None                          # lspci -v
> >   Memory behind bridge: None                          # lspci -vv
> >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> >
> > NEW output for -v and -vv to also use "[empty]":
> >
> >   Memory behind bridge: [empty]                       # lspci -v
> >   Memory behind bridge: [empty]                       # lspci -vv
> >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> 
> How about this alternative?  The spec doesn't actually use the term
> "window", but I think in terms of bridge windows to downstream
> devices, and the windows can be either enabled or disabled.
> 
>   Memory behind bridge: Disabled    # lspci -v or lspci -vv
>   Memory behind bridge: Disabled [0x0000e000-0x0000efff]   # lspci -vvv
> 

I like the alternative of using "Disabled". Could it be more suiting to use
"[disabled]" which is also used in show_bases(), show_rom(), and show_htype2()?
Then keep the range format the same. For example:

   Memory behind bridge: [disabled]    # lspci -v or lspci -vv
   Memory behind bridge: 0x0000e000-0x0000efff [disabled]   # lspci -vvv

I would be happy to submit an updated patch for either version thought to read
better.

> > Advantage is consistent output regardless of verbosity level chosen and
> > to simplify the code.
> >
> > Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> > ---
> >  lspci.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/lspci.c b/lspci.c
> > index 7418b07..0c00c91 100644
> > --- a/lspci.c
> > +++ b/lspci.c
> > @@ -376,16 +376,14 @@ 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
> > --
> > 2.20.1
> >

[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[bjorn.helgaas]

On Mon, Jun 17, 2019 at 5:29 PM Kelsey Skunberg
<skunberg.kelsey at gmail.com> wrote:
> On Tue, Jun 11, 2019 at 03:25:00PM -0500, Bjorn Helgaas wrote:
> > On Thu, May 16, 2019 at 4:23 PM Kelsey Skunberg
> > <skunberg.kelsey at gmail.com> wrote:
> > >
> > > Change output displayed for memory behind bridge when the range is
> > > empty to be consistent between each verbosity level. Replace 'None' with
> > > '[empty]'. 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 ouptut for -v and -vv which uses 'None' and -vvv uses '[empty]':
> >
> > s/ouptut/output/
> >
> > >   Memory behind bridge: None                          # lspci -v
> > >   Memory behind bridge: None                          # lspci -vv
> > >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> > >
> > > NEW output for -v and -vv to also use "[empty]":
> > >
> > >   Memory behind bridge: [empty]                       # lspci -v
> > >   Memory behind bridge: [empty]                       # lspci -vv
> > >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> >
> > How about this alternative?  The spec doesn't actually use the term
> > "window", but I think in terms of bridge windows to downstream
> > devices, and the windows can be either enabled or disabled.
> >
> >   Memory behind bridge: Disabled    # lspci -v or lspci -vv
> >   Memory behind bridge: Disabled [0x0000e000-0x0000efff]   # lspci -vvv
> >
>
> I like the alternative of using "Disabled". Could it be more suiting to use
> "[disabled]" which is also used in show_bases(), show_rom(), and show_htype2()?
> Then keep the range format the same. For example:
>
>    Memory behind bridge: [disabled]    # lspci -v or lspci -vv
>    Memory behind bridge: 0x0000e000-0x0000efff [disabled]   # lspci -vvv

I like it, and I like your attention to detail in matching other parts
of lspci output.

> > > Advantage is consistent output regardless of verbosity level chosen and
> > > to simplify the code.
> > >
> > > Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> > > ---
> > >  lspci.c | 14 ++++++--------
> > >  1 file changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/lspci.c b/lspci.c
> > > index 7418b07..0c00c91 100644
> > > --- a/lspci.c
> > > +++ b/lspci.c
> > > @@ -376,16 +376,14 @@ 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
> > > --
> > > 2.20.1
> > >

[Linux-kernel-mentees] [PATCH v3 3/3] lspci: Replace output for bridge with empty range from 'None' to '[empty]'

[Bjorn Helgaas]

On Mon, Jun 17, 2019 at 5:29 PM Kelsey Skunberg
<skunberg.kelsey at gmail.com> wrote:
> On Tue, Jun 11, 2019 at 03:25:00PM -0500, Bjorn Helgaas wrote:
> > On Thu, May 16, 2019 at 4:23 PM Kelsey Skunberg
> > <skunberg.kelsey at gmail.com> wrote:
> > >
> > > Change output displayed for memory behind bridge when the range is
> > > empty to be consistent between each verbosity level. Replace 'None' with
> > > '[empty]'. 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 ouptut for -v and -vv which uses 'None' and -vvv uses '[empty]':
> >
> > s/ouptut/output/
> >
> > >   Memory behind bridge: None                          # lspci -v
> > >   Memory behind bridge: None                          # lspci -vv
> > >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> > >
> > > NEW output for -v and -vv to also use "[empty]":
> > >
> > >   Memory behind bridge: [empty]                       # lspci -v
> > >   Memory behind bridge: [empty]                       # lspci -vv
> > >   Memory behind bridge: 0000e000-0000efff [empty]     # lspci -vvv
> >
> > How about this alternative?  The spec doesn't actually use the term
> > "window", but I think in terms of bridge windows to downstream
> > devices, and the windows can be either enabled or disabled.
> >
> >   Memory behind bridge: Disabled    # lspci -v or lspci -vv
> >   Memory behind bridge: Disabled [0x0000e000-0x0000efff]   # lspci -vvv
> >
>
> I like the alternative of using "Disabled". Could it be more suiting to use
> "[disabled]" which is also used in show_bases(), show_rom(), and show_htype2()?
> Then keep the range format the same. For example:
>
>    Memory behind bridge: [disabled]    # lspci -v or lspci -vv
>    Memory behind bridge: 0x0000e000-0x0000efff [disabled]   # lspci -vvv

I like it, and I like your attention to detail in matching other parts
of lspci output.

> > > Advantage is consistent output regardless of verbosity level chosen and
> > > to simplify the code.
> > >
> > > Signed-off-by: Kelsey Skunberg <skunberg.kelsey at gmail.com>
> > > ---
> > >  lspci.c | 14 ++++++--------
> > >  1 file changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/lspci.c b/lspci.c
> > > index 7418b07..0c00c91 100644
> > > --- a/lspci.c
> > > +++ b/lspci.c
> > > @@ -376,16 +376,14 @@ 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
> > > --
> > > 2.20.1
> > >