If I configure my build with --enable-sanitizers, my GCC (v8.5.0)
complains:
.../softmmu/device_tree.c: In function ‘qemu_fdt_add_path’:
.../softmmu/device_tree.c:560:18: error: ‘retval’ may be used uninitialized
in this function [-Werror=maybe-uninitialized]
int namelen, retval;
^~~~~~
It's a false warning since the while loop is always executed at least
once (p has to be non-NULL, otherwise the derefence in the if-statement
earlier will crash). Thus let's switch to a do-while loop here instead
to make the compiler happy in all cases.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
softmmu/device_tree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 3965c834ca..9e96f5ecd5 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -564,7 +564,7 @@ int qemu_fdt_add_path(void *fdt, const char *path)
return -1;
}
- while (p) {
+ do {
name = p + 1;
p = strchr(name, '/');
namelen = p != NULL ? p - name : strlen(name);
@@ -584,7 +584,7 @@ int qemu_fdt_add_path(void *fdt, const char *path)
}
parent = retval;
- }
+ } while (p);
return retval;
}
--
2.27.0
On Fri, Jan 07, 2022 at 02:38:44PM +0100, Thomas Huth wrote: > If I configure my build with --enable-sanitizers, my GCC (v8.5.0) > complains: > > .../softmmu/device_tree.c: In function ‘qemu_fdt_add_path’: > .../softmmu/device_tree.c:560:18: error: ‘retval’ may be used uninitialized > in this function [-Werror=maybe-uninitialized] > int namelen, retval; > ^~~~~~ > > It's a false warning since the while loop is always executed at least > once (p has to be non-NULL, otherwise the derefence in the if-statement > earlier will crash). Thus let's switch to a do-while loop here instead > to make the compiler happy in all cases. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > softmmu/device_tree.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index 3965c834ca..9e96f5ecd5 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -564,7 +564,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > return -1; > } > > - while (p) { > + do { > name = p + 1; > p = strchr(name, '/'); > namelen = p != NULL ? p - name : strlen(name); > @@ -584,7 +584,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > } > > parent = retval; > - } > + } while (p); > > return retval; > } > -- > 2.27.0 > Reviewed-by: Andrew Jones <drjones@redhat.com>
On 7/1/22 14:38, Thomas Huth wrote: > If I configure my build with --enable-sanitizers, my GCC (v8.5.0) > complains: > > .../softmmu/device_tree.c: In function ‘qemu_fdt_add_path’: > .../softmmu/device_tree.c:560:18: error: ‘retval’ may be used uninitialized > in this function [-Werror=maybe-uninitialized] > int namelen, retval; > ^~~~~~ > > It's a false warning since the while loop is always executed at least > once (p has to be non-NULL, otherwise the derefence in the if-statement > earlier will crash). Thus let's switch to a do-while loop here instead > to make the compiler happy in all cases. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > softmmu/device_tree.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On 1/7/22 5:38 AM, Thomas Huth wrote: > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index 3965c834ca..9e96f5ecd5 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -564,7 +564,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > return -1; > } > > - while (p) { > + do { > name = p + 1; > p = strchr(name, '/'); > namelen = p != NULL ? p - name : strlen(name); > @@ -584,7 +584,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > } > > parent = retval; > - } > + } while (p); Reviewed-by: Richard Henderson <richard.henderson@linaro.org> In addition, the copy between 'path' and 'p' is unnecessary -- one of the variables should be removed. Either rename the parameter to 'p', or rename all uses to 'path'. r~
On 2022/1/8 4:18, Richard Henderson wrote: > On 1/7/22 5:38 AM, Thomas Huth wrote: >> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c >> index 3965c834ca..9e96f5ecd5 100644 >> --- a/softmmu/device_tree.c >> +++ b/softmmu/device_tree.c >> @@ -564,7 +564,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) >> return -1; >> } >> - while (p) { >> + do { >> name = p + 1; >> p = strchr(name, '/'); >> namelen = p != NULL ? p - name : strlen(name); >> @@ -584,7 +584,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) >> } >> parent = retval; >> - } >> + } while (p); > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > In addition, the copy between 'path' and 'p' is unnecessary -- one of > the variables should be removed. Either rename the parameter to 'p', > or rename all uses to 'path'. > Yes. We can simply remove "const char *p = path;" and directly using the parameter "path" passed in would be fine enough. Thanks for catching this. Thanks, Yanan > > r~ > .
On Fri, Jan 7, 2022 at 11:41 PM Thomas Huth <thuth@redhat.com> wrote: > > If I configure my build with --enable-sanitizers, my GCC (v8.5.0) > complains: > > .../softmmu/device_tree.c: In function ‘qemu_fdt_add_path’: > .../softmmu/device_tree.c:560:18: error: ‘retval’ may be used uninitialized > in this function [-Werror=maybe-uninitialized] > int namelen, retval; > ^~~~~~ > > It's a false warning since the while loop is always executed at least > once (p has to be non-NULL, otherwise the derefence in the if-statement > earlier will crash). Thus let's switch to a do-while loop here instead > to make the compiler happy in all cases. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > softmmu/device_tree.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index 3965c834ca..9e96f5ecd5 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -564,7 +564,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > return -1; > } > > - while (p) { > + do { > name = p + 1; > p = strchr(name, '/'); > namelen = p != NULL ? p - name : strlen(name); > @@ -584,7 +584,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > } > > parent = retval; > - } > + } while (p); > > return retval; > } > -- > 2.27.0 > >
On 2022/1/7 21:38, Thomas Huth wrote: > If I configure my build with --enable-sanitizers, my GCC (v8.5.0) > complains: > > .../softmmu/device_tree.c: In function ‘qemu_fdt_add_path’: > .../softmmu/device_tree.c:560:18: error: ‘retval’ may be used uninitialized > in this function [-Werror=maybe-uninitialized] > int namelen, retval; > ^~~~~~ > > It's a false warning since the while loop is always executed at least > once (p has to be non-NULL, otherwise the derefence in the if-statement > earlier will crash). Thus let's switch to a do-while loop here instead > to make the compiler happy in all cases. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > softmmu/device_tree.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Yanan Wang <wangyanan55@huawei.com> > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index 3965c834ca..9e96f5ecd5 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -564,7 +564,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > return -1; > } > > - while (p) { > + do { > name = p + 1; > p = strchr(name, '/'); > namelen = p != NULL ? p - name : strlen(name); > @@ -584,7 +584,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > } > > parent = retval; > - } > + } while (p); > > return retval; > }
On Fri, Jan 7, 2022 at 11:41 PM Thomas Huth <thuth@redhat.com> wrote: > > If I configure my build with --enable-sanitizers, my GCC (v8.5.0) > complains: > > .../softmmu/device_tree.c: In function ‘qemu_fdt_add_path’: > .../softmmu/device_tree.c:560:18: error: ‘retval’ may be used uninitialized > in this function [-Werror=maybe-uninitialized] > int namelen, retval; > ^~~~~~ > > It's a false warning since the while loop is always executed at least > once (p has to be non-NULL, otherwise the derefence in the if-statement > earlier will crash). Thus let's switch to a do-while loop here instead > to make the compiler happy in all cases. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Thanks! Applied to riscv-to-apply.next If anyone else (trivial maybe?) wants to take this feel free to as well, otherwise I'll just take this via the RISC-V tree. Alistair > --- > softmmu/device_tree.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index 3965c834ca..9e96f5ecd5 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -564,7 +564,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > return -1; > } > > - while (p) { > + do { > name = p + 1; > p = strchr(name, '/'); > namelen = p != NULL ? p - name : strlen(name); > @@ -584,7 +584,7 @@ int qemu_fdt_add_path(void *fdt, const char *path) > } > > parent = retval; > - } > + } while (p); > > return retval; > } > -- > 2.27.0 > >
© 2016 - 2024 Red Hat, Inc.