* [PATCH] device_tree: Fix compiler error
@ 2021-11-08 20:07 Stefan Weil
2021-11-08 22:43 ` Alistair Francis
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stefan Weil @ 2021-11-08 20:07 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-trivial, Richard Henderson, Stefan Weil, Alistair Francis,
David Gibson
A build with gcc (Debian 10.2.1-6) 10.2.1 20210110 fails:
../../../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]
560 | int namelen, retval;
| ^~~~~~
This is not a real error, but the compiler can be satisfied with a small change.
Fixes: b863f0b75852 ("device_tree: Add qemu_fdt_add_path")
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
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.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] device_tree: Fix compiler error
2021-11-08 20:07 [PATCH] device_tree: Fix compiler error Stefan Weil
@ 2021-11-08 22:43 ` Alistair Francis
2021-11-09 7:58 ` Stefan Weil
2021-11-09 8:38 ` Richard Henderson
2021-12-17 10:11 ` Laurent Vivier
2 siblings, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2021-11-08 22:43 UTC (permalink / raw)
To: Stefan Weil
Cc: QEMU Trivial, Alistair Francis, Richard Henderson,
qemu-devel@nongnu.org Developers, David Gibson
On Tue, Nov 9, 2021 at 6:08 AM Stefan Weil <sw@weilnetz.de> wrote:
>
> A build with gcc (Debian 10.2.1-6) 10.2.1 20210110 fails:
>
> ../../../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]
> 560 | int namelen, retval;
> | ^~~~~~
>
> This is not a real error, but the compiler can be satisfied with a small change.
Why don't we just initalise retval?
Alistair
>
> Fixes: b863f0b75852 ("device_tree: Add qemu_fdt_add_path")
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 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.30.2
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] device_tree: Fix compiler error
2021-11-08 22:43 ` Alistair Francis
@ 2021-11-09 7:58 ` Stefan Weil
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Weil @ 2021-11-09 7:58 UTC (permalink / raw)
To: Alistair Francis
Cc: QEMU Trivial, Alistair Francis, Richard Henderson,
qemu-devel@nongnu.org Developers, David Gibson
Am 08.11.21 um 23:43 schrieb Alistair Francis:
> On Tue, Nov 9, 2021 at 6:08 AM Stefan Weil <sw@weilnetz.de> wrote:
>> A build with gcc (Debian 10.2.1-6) 10.2.1 20210110 fails:
>>
>> ../../../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]
>> 560 | int namelen, retval;
>> | ^~~~~~
>>
>> This is not a real error, but the compiler can be satisfied with a small change.
> Why don't we just initalise retval?
>
> Alistair
retval is already set in the do ... while loop and was also set in the
former while loop.
If we set it in the declaration (int retval = 0), a clever compiler
might complain that we assign a value which is never used.
And changing from the while loop to the do ... while loop also avoids
one compare statement, so depending on the compiler optimizations could
make the code a little bit faster.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] device_tree: Fix compiler error
2021-11-08 20:07 [PATCH] device_tree: Fix compiler error Stefan Weil
2021-11-08 22:43 ` Alistair Francis
@ 2021-11-09 8:38 ` Richard Henderson
2021-11-09 8:46 ` Michal Prívozník
2021-12-17 10:11 ` Laurent Vivier
2 siblings, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2021-11-09 8:38 UTC (permalink / raw)
To: Stefan Weil, qemu-devel; +Cc: qemu-trivial, Alistair Francis, David Gibson
On 11/8/21 9:07 PM, Stefan Weil wrote:
> A build with gcc (Debian 10.2.1-6) 10.2.1 20210110 fails:
>
> ../../../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]
> 560 | int namelen, retval;
> | ^~~~~~
>
> This is not a real error, but the compiler can be satisfied with a small change.
>
> Fixes: b863f0b75852 ("device_tree: Add qemu_fdt_add_path")
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Though I think there's a good deal that could be cleaned up about this function:
(1a) Remove the unused return value?
The single use does not check the return.
(1b) Don't attempt to return a node, merely a success/failure code.
Certainly the local documentation here could be improved...
(1c) Return parent; make retval local to the loop.
(2) Merge p and path; there's no point retaining the unmodified parameter.
(3) Move name and namelen inside the loop.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] device_tree: Fix compiler error
2021-11-09 8:38 ` Richard Henderson
@ 2021-11-09 8:46 ` Michal Prívozník
0 siblings, 0 replies; 6+ messages in thread
From: Michal Prívozník @ 2021-11-09 8:46 UTC (permalink / raw)
To: Richard Henderson, Stefan Weil, qemu-devel
Cc: qemu-trivial, Alistair Francis, David Gibson
On 11/9/21 9:38 AM, Richard Henderson wrote:
> On 11/8/21 9:07 PM, Stefan Weil wrote:
>> A build with gcc (Debian 10.2.1-6) 10.2.1 20210110 fails:
>>
>> ../../../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]
>> 560 | int namelen, retval;
>> | ^~~~~~
>>
>> This is not a real error, but the compiler can be satisfied with a
>> small change.
>>
>> Fixes: b863f0b75852 ("device_tree: Add qemu_fdt_add_path")
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> Though I think there's a good deal that could be cleaned up about this
> function:
>
> (1a) Remove the unused return value?
> The single use does not check the return.
>
> (1b) Don't attempt to return a node, merely a success/failure code.
> Certainly the local documentation here could be improved...
>
> (1c) Return parent; make retval local to the loop.
>
> (2) Merge p and path; there's no point retaining the unmodified parameter.
>
> (3) Move name and namelen inside the loop.
(4) swap if() bodies?
if (retval == -FDT_ERR_NOTFOUND) {
} else if (retval < 0) {
}
Michal
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] device_tree: Fix compiler error
2021-11-08 20:07 [PATCH] device_tree: Fix compiler error Stefan Weil
2021-11-08 22:43 ` Alistair Francis
2021-11-09 8:38 ` Richard Henderson
@ 2021-12-17 10:11 ` Laurent Vivier
2 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2021-12-17 10:11 UTC (permalink / raw)
To: Stefan Weil, qemu-devel
Cc: qemu-trivial, Alistair Francis, Richard Henderson, David Gibson
Le 08/11/2021 à 21:07, Stefan Weil a écrit :
> A build with gcc (Debian 10.2.1-6) 10.2.1 20210110 fails:
>
> ../../../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]
> 560 | int namelen, retval;
> | ^~~~~~
>
> This is not a real error, but the compiler can be satisfied with a small change.
>
> Fixes: b863f0b75852 ("device_tree: Add qemu_fdt_add_path")
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 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;
> }
>
I think to add a "g_assert_nonull(p);" before the loop would inform the compiler that we always go
in the loop and remove the compiler error.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-17 10:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-08 20:07 [PATCH] device_tree: Fix compiler error Stefan Weil
2021-11-08 22:43 ` Alistair Francis
2021-11-09 7:58 ` Stefan Weil
2021-11-09 8:38 ` Richard Henderson
2021-11-09 8:46 ` Michal Prívozník
2021-12-17 10:11 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.