| Message ID | 20240731143617.3391947-3-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Miscellaneous minor Coverity fixes | expand |
Am 31.07.2024 um 16:36 hat Peter Maydell geschrieben: > In the loop in qemu_gluster_parse_json() we do: > > char *str = NULL; > for(...) { > str = g_strdup_printf(...); > ... > if (various errors) { > goto out; > } > ... > g_free(str); > str = NULL; > } > return 0; > out: > various cleanups; > g_free(str); > ... > return -errno; > > Coverity correctly complains that the assignment "str = NULL" at the > end of the loop is unnecessary, because we will either go back to the > top of the loop and overwrite it, or else we will exit the loop and > then exit the function without ever reading str again. The assignment > is there as defensive coding to ensure that str is only non-NULL if > it's a live allocation, so this is intentional. > > We can make Coverity happier and simplify the code here by using > g_autofree, since we never need 'str' outside the loop. > > Resolves: Coverity CID 1527385 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > block/gluster.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/block/gluster.c b/block/gluster.c > index f8b415f3812..61ded95e660 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -514,7 +514,6 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, > SocketAddressList **tail; > QDict *backing_options = NULL; > Error *local_err = NULL; > - char *str = NULL; > const char *ptr; > int i, type, num_servers; > > @@ -547,7 +546,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, > tail = &gconf->server; > > for (i = 0; i < num_servers; i++) { > - str = g_strdup_printf(GLUSTER_OPT_SERVER_PATTERN"%d.", i); > + g_autofree char *str = g_strdup_printf(GLUSTER_OPT_SERVER_PATTERN"%d.", i); This line is too long now. With this fixed: Reviewed-by: Kevin Wolf <kwolf@redhat.com>
On 31/7/24 16:36, Peter Maydell wrote: > In the loop in qemu_gluster_parse_json() we do: > > char *str = NULL; > for(...) { > str = g_strdup_printf(...); > ... > if (various errors) { > goto out; > } > ... > g_free(str); > str = NULL; > } > return 0; > out: > various cleanups; > g_free(str); > ... > return -errno; > > Coverity correctly complains that the assignment "str = NULL" at the > end of the loop is unnecessary, because we will either go back to the > top of the loop and overwrite it, or else we will exit the loop and > then exit the function without ever reading str again. The assignment > is there as defensive coding to ensure that str is only non-NULL if > it's a live allocation, so this is intentional. > > We can make Coverity happier and simplify the code here by using > g_autofree, since we never need 'str' outside the loop. > > Resolves: Coverity CID 1527385 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > block/gluster.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/block/gluster.c b/block/gluster.c index f8b415f3812..61ded95e660 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -514,7 +514,6 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, SocketAddressList **tail; QDict *backing_options = NULL; Error *local_err = NULL; - char *str = NULL; const char *ptr; int i, type, num_servers; @@ -547,7 +546,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, tail = &gconf->server; for (i = 0; i < num_servers; i++) { - str = g_strdup_printf(GLUSTER_OPT_SERVER_PATTERN"%d.", i); + g_autofree char *str = g_strdup_printf(GLUSTER_OPT_SERVER_PATTERN"%d.", i); qdict_extract_subqdict(options, &backing_options, str); /* create opts info from runtime_type_opts list */ @@ -658,8 +657,6 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, qobject_unref(backing_options); backing_options = NULL; - g_free(str); - str = NULL; } return 0; @@ -668,7 +665,6 @@ out: error_propagate(errp, local_err); qapi_free_SocketAddress(gsconf); qemu_opts_del(opts); - g_free(str); qobject_unref(backing_options); errno = EINVAL; return -errno;
In the loop in qemu_gluster_parse_json() we do: char *str = NULL; for(...) { str = g_strdup_printf(...); ... if (various errors) { goto out; } ... g_free(str); str = NULL; } return 0; out: various cleanups; g_free(str); ... return -errno; Coverity correctly complains that the assignment "str = NULL" at the end of the loop is unnecessary, because we will either go back to the top of the loop and overwrite it, or else we will exit the loop and then exit the function without ever reading str again. The assignment is there as defensive coding to ensure that str is only non-NULL if it's a live allocation, so this is intentional. We can make Coverity happier and simplify the code here by using g_autofree, since we never need 'str' outside the loop. Resolves: Coverity CID 1527385 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- block/gluster.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)