diff mbox series

[XEN,RFC,01/40] tools: Fix -Werror=maybe-uninitialized for xlu_pci_parse_bdf

Message ID 20210811102423.28908-2-wei.chen@arm.com (mailing list archive)
State New, archived
Headers show
Series Add device tree based NUMA support to Arm64 | expand

Commit Message

Wei Chen Aug. 11, 2021, 10:23 a.m. UTC
| libxlu_pci.c: In function 'xlu_pci_parse_bdf':
| libxlu_pci.c:32:18: error: 'func' may be used uninitialized in this function [-Werror=maybe-uninitialized]
|    32 |     pcidev->func = func;
|       |     ~~~~~~~~~~~~~^~~~~~
| libxlu_pci.c:51:29: note: 'func' was declared here
|    51 |     unsigned dom, bus, dev, func, vslot = 0;
|       |                             ^~~~
| libxlu_pci.c:31:17: error: 'dev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
|    31 |     pcidev->dev = dev;
|       |     ~~~~~~~~~~~~^~~~~
| libxlu_pci.c:51:24: note: 'dev' was declared here
|    51 |     unsigned dom, bus, dev, func, vslot = 0;
|       |                        ^~~
| libxlu_pci.c:30:17: error: 'bus' may be used uninitialized in this function [-Werror=maybe-uninitialized]
|    30 |     pcidev->bus = bus;
|       |     ~~~~~~~~~~~~^~~~~
| libxlu_pci.c:51:19: note: 'bus' was declared here
|    51 |     unsigned dom, bus, dev, func, vslot = 0;
|       |                   ^~~
| libxlu_pci.c:78:26: error: 'dom' may be used uninitialized in this function [-Werror=maybe-uninitialized]
|    78 |                 if ( dom & ~0xff )
|       |                      ~~~~^~~~~~~

Signed-off-by: Wei Chen <wei.chen@arm.com>
---
 tools/libs/util/libxlu_pci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jan Beulich Aug. 11, 2021, 10:49 a.m. UTC | #1
On 11.08.2021 12:23, Wei Chen wrote:
> | libxlu_pci.c: In function 'xlu_pci_parse_bdf':
> | libxlu_pci.c:32:18: error: 'func' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> |    32 |     pcidev->func = func;
> |       |     ~~~~~~~~~~~~~^~~~~~

I'm afraid I can't spot such an assignment in the file (nor the two
similar ones further down). All I can see is 

    pci->domain = domain;
    pci->bus = bus;
    pci->dev = dev;
    pci->func = func;

> | libxlu_pci.c:51:29: note: 'func' was declared here
> |    51 |     unsigned dom, bus, dev, func, vslot = 0;
> |       |                             ^~~~
> | libxlu_pci.c:31:17: error: 'dev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> |    31 |     pcidev->dev = dev;
> |       |     ~~~~~~~~~~~~^~~~~
> | libxlu_pci.c:51:24: note: 'dev' was declared here
> |    51 |     unsigned dom, bus, dev, func, vslot = 0;
> |       |                        ^~~
> | libxlu_pci.c:30:17: error: 'bus' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> |    30 |     pcidev->bus = bus;
> |       |     ~~~~~~~~~~~~^~~~~
> | libxlu_pci.c:51:19: note: 'bus' was declared here
> |    51 |     unsigned dom, bus, dev, func, vslot = 0;
> |       |                   ^~~
> | libxlu_pci.c:78:26: error: 'dom' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> |    78 |                 if ( dom & ~0xff )
> |       |                      ~~~~^~~~~~~

I'm afraid I also can't spot a variable named "dom", nor a sufficiently
similar if(). May I ask what code base these were observed with? Is the
change needed at all anymore?

> --- a/tools/libs/util/libxlu_pci.c
> +++ b/tools/libs/util/libxlu_pci.c
> @@ -15,7 +15,7 @@ static int parse_bdf(libxl_device_pci *pci, const char *str, const char **endp)
>  {
>      const char *ptr = str;
>      unsigned int colons = 0;
> -    unsigned int domain, bus, dev, func;
> +    unsigned int domain = 0, bus = 0, dev = 0, func = 0;
>      int n;
>  
>      /* Count occurrences of ':' to detrmine presence/absence of the 'domain' */
> @@ -28,7 +28,6 @@ static int parse_bdf(libxl_device_pci *pci, const char *str, const char **endp)
>      ptr = str;
>      switch (colons) {
>      case 1:
> -        domain = 0;
>          if (sscanf(ptr, "%x:%x.%n", &bus, &dev, &n) != 2)
>              return ERROR_INVAL;
>          break;
> 

Also - which compiler did you encounter this with?

Finally please don't forget to Cc maintainers.

Jan
Wei Chen Aug. 13, 2021, 6:28 a.m. UTC | #2
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2021年8月11日 18:50
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen-
> devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org
> Subject: Re: [XEN RFC PATCH 01/40] tools: Fix -Werror=maybe-uninitialized
> for xlu_pci_parse_bdf
> 
> On 11.08.2021 12:23, Wei Chen wrote:
> > | libxlu_pci.c: In function 'xlu_pci_parse_bdf':
> > | libxlu_pci.c:32:18: error: 'func' may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> > |    32 |     pcidev->func = func;
> > |       |     ~~~~~~~~~~~~~^~~~~~
> 
> I'm afraid I can't spot such an assignment in the file (nor the two
> similar ones further down). All I can see is
> 
>     pci->domain = domain;
>     pci->bus = bus;
>     pci->dev = dev;
>     pci->func = func;
> 

Sorry, I forgot to update my commit log with the latest code base.
I revert this change in my current code, I can't reproduce it.
I'm not sure if it's because I upgraded my build environment.
Give me sometime, if I can reproduce it I will update the commit
log in next version. If it's no longer needed, I will remove this
patch from this series.

> > | libxlu_pci.c:51:29: note: 'func' was declared here
> > |    51 |     unsigned dom, bus, dev, func, vslot = 0;
> > |       |                             ^~~~
> > | libxlu_pci.c:31:17: error: 'dev' may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> > |    31 |     pcidev->dev = dev;
> > |       |     ~~~~~~~~~~~~^~~~~
> > | libxlu_pci.c:51:24: note: 'dev' was declared here
> > |    51 |     unsigned dom, bus, dev, func, vslot = 0;
> > |       |                        ^~~
> > | libxlu_pci.c:30:17: error: 'bus' may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> > |    30 |     pcidev->bus = bus;
> > |       |     ~~~~~~~~~~~~^~~~~
> > | libxlu_pci.c:51:19: note: 'bus' was declared here
> > |    51 |     unsigned dom, bus, dev, func, vslot = 0;
> > |       |                   ^~~
> > | libxlu_pci.c:78:26: error: 'dom' may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> > |    78 |                 if ( dom & ~0xff )
> > |       |                      ~~~~^~~~~~~
> 
> I'm afraid I also can't spot a variable named "dom", nor a sufficiently
> similar if(). May I ask what code base these were observed with? Is the
> change needed at all anymore?
> 

same as above.

> > --- a/tools/libs/util/libxlu_pci.c
> > +++ b/tools/libs/util/libxlu_pci.c
> > @@ -15,7 +15,7 @@ static int parse_bdf(libxl_device_pci *pci, const char
> *str, const char **endp)
> >  {
> >      const char *ptr = str;
> >      unsigned int colons = 0;
> > -    unsigned int domain, bus, dev, func;
> > +    unsigned int domain = 0, bus = 0, dev = 0, func = 0;
> >      int n;
> >
> >      /* Count occurrences of ':' to detrmine presence/absence of the
> 'domain' */
> > @@ -28,7 +28,6 @@ static int parse_bdf(libxl_device_pci *pci, const char
> *str, const char **endp)
> >      ptr = str;
> >      switch (colons) {
> >      case 1:
> > -        domain = 0;
> >          if (sscanf(ptr, "%x:%x.%n", &bus, &dev, &n) != 2)
> >              return ERROR_INVAL;
> >          break;
> >
> 
> Also - which compiler did you encounter this with?
> 
> Finally please don't forget to Cc maintainers.
> 

If this patch still needed, I will do in next version.

> Jan
diff mbox series

Patch

diff --git a/tools/libs/util/libxlu_pci.c b/tools/libs/util/libxlu_pci.c
index 551d8e3aed..b38e9aab40 100644
--- a/tools/libs/util/libxlu_pci.c
+++ b/tools/libs/util/libxlu_pci.c
@@ -15,7 +15,7 @@  static int parse_bdf(libxl_device_pci *pci, const char *str, const char **endp)
 {
     const char *ptr = str;
     unsigned int colons = 0;
-    unsigned int domain, bus, dev, func;
+    unsigned int domain = 0, bus = 0, dev = 0, func = 0;
     int n;
 
     /* Count occurrences of ':' to detrmine presence/absence of the 'domain' */
@@ -28,7 +28,6 @@  static int parse_bdf(libxl_device_pci *pci, const char *str, const char **endp)
     ptr = str;
     switch (colons) {
     case 1:
-        domain = 0;
         if (sscanf(ptr, "%x:%x.%n", &bus, &dev, &n) != 2)
             return ERROR_INVAL;
         break;