* [PATCH 0/2] Prevent uninitialized warnings @ 2020-01-21 9:28 mrezanin 2020-01-21 9:28 ` [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning mrezanin 2020-01-21 9:28 ` [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning mrezanin 0 siblings, 2 replies; 10+ messages in thread From: mrezanin @ 2020-01-21 9:28 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell From: Miroslav Rezanina <mrezanin@redhat.com> Using -Wmaybe-uninitialized when optimalization is enabled can cause several warnings during build. This will break build in case -Werror is used. This series fixes two cases of this warnings that can happen during build of QEMU. Miroslav Rezanina (2): test-logging: Fix -Werror=maybe-uninitialized warning aspeed/i2c: Prevent uninitialized warning hw/i2c/aspeed_i2c.c | 2 +- tests/test-logging.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning 2020-01-21 9:28 [PATCH 0/2] Prevent uninitialized warnings mrezanin @ 2020-01-21 9:28 ` mrezanin 2020-01-21 9:58 ` Thomas Huth 2020-01-21 9:28 ` [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning mrezanin 1 sibling, 1 reply; 10+ messages in thread From: mrezanin @ 2020-01-21 9:28 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell From: Miroslav Rezanina <mrezanin@redhat.com> Checking for uninitialized variables raises warning for file path variables in test_logfile_write and test_logfile_lock functions. To suppress this warning, initialize varibles to NULL. This is safe change as result of g_build_filename is stored to them before any usage. Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> --- tests/test-logging.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test-logging.c b/tests/test-logging.c index 1e646f0..6387e49 100644 --- a/tests/test-logging.c +++ b/tests/test-logging.c @@ -114,8 +114,8 @@ static void test_logfile_write(gconstpointer data) QemuLogFile *logfile2; gchar const *dir = data; Error *err = NULL; - g_autofree gchar *file_path; - g_autofree gchar *file_path1; + g_autofree gchar *file_path = NULL; + g_autofree gchar *file_path1 = NULL; FILE *orig_fd; /* @@ -157,7 +157,7 @@ static void test_logfile_lock(gconstpointer data) FILE *logfile; gchar const *dir = data; Error *err = NULL; - g_autofree gchar *file_path; + g_autofree gchar *file_path = NULL; file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning 2020-01-21 9:28 ` [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning mrezanin @ 2020-01-21 9:58 ` Thomas Huth 2020-01-21 15:03 ` Robert Foley 0 siblings, 1 reply; 10+ messages in thread From: Thomas Huth @ 2020-01-21 9:58 UTC (permalink / raw) To: mrezanin, qemu-devel Cc: qemu-trivial, peter.maydell, Alex Bennée, Robert Foley On 21/01/2020 10.28, mrezanin@redhat.com wrote: > From: Miroslav Rezanina <mrezanin@redhat.com> > > Checking for uninitialized variables raises warning for file path > variables in test_logfile_write and test_logfile_lock functions. > > To suppress this warning, initialize varibles to NULL. This is safe > change as result of g_build_filename is stored to them before any usage. > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > --- > tests/test-logging.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/test-logging.c b/tests/test-logging.c > index 1e646f0..6387e49 100644 > --- a/tests/test-logging.c > +++ b/tests/test-logging.c > @@ -114,8 +114,8 @@ static void test_logfile_write(gconstpointer data) > QemuLogFile *logfile2; > gchar const *dir = data; > Error *err = NULL; > - g_autofree gchar *file_path; > - g_autofree gchar *file_path1; > + g_autofree gchar *file_path = NULL; > + g_autofree gchar *file_path1 = NULL; > FILE *orig_fd; > > /* > @@ -157,7 +157,7 @@ static void test_logfile_lock(gconstpointer data) > FILE *logfile; > gchar const *dir = data; > Error *err = NULL; > - g_autofree gchar *file_path; > + g_autofree gchar *file_path = NULL; > > file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL); Right. The glib documentation clearly states that "the variable must be initialized", see: https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#g-autofree So this is the right thing to do here! Reviewed-by: Thomas Huth <thuth@redhat.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning 2020-01-21 9:58 ` Thomas Huth @ 2020-01-21 15:03 ` Robert Foley 0 siblings, 0 replies; 10+ messages in thread From: Robert Foley @ 2020-01-21 15:03 UTC (permalink / raw) To: Thomas Huth Cc: qemu-trivial, peter.maydell, mrezanin, Alex Bennée, qemu-devel Good catch. Reviewed-by: Robert Foley <robert.foley@linaro.org> On Tue, 21 Jan 2020 at 04:58, Thomas Huth <thuth@redhat.com> wrote: > > On 21/01/2020 10.28, mrezanin@redhat.com wrote: > > From: Miroslav Rezanina <mrezanin@redhat.com> > > > > Checking for uninitialized variables raises warning for file path > > variables in test_logfile_write and test_logfile_lock functions. > > > > To suppress this warning, initialize varibles to NULL. This is safe > > change as result of g_build_filename is stored to them before any usage. > > > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > > --- > > tests/test-logging.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tests/test-logging.c b/tests/test-logging.c > > index 1e646f0..6387e49 100644 > > --- a/tests/test-logging.c > > +++ b/tests/test-logging.c > > @@ -114,8 +114,8 @@ static void test_logfile_write(gconstpointer data) > > QemuLogFile *logfile2; > > gchar const *dir = data; > > Error *err = NULL; > > - g_autofree gchar *file_path; > > - g_autofree gchar *file_path1; > > + g_autofree gchar *file_path = NULL; > > + g_autofree gchar *file_path1 = NULL; > > FILE *orig_fd; > > > > /* > > @@ -157,7 +157,7 @@ static void test_logfile_lock(gconstpointer data) > > FILE *logfile; > > gchar const *dir = data; > > Error *err = NULL; > > - g_autofree gchar *file_path; > > + g_autofree gchar *file_path = NULL; > > > > file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL); > > Right. The glib documentation clearly states that "the variable must be > initialized", see: > > https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#g-autofree > > So this is the right thing to do here! > > Reviewed-by: Thomas Huth <thuth@redhat.com> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning 2020-01-21 9:28 [PATCH 0/2] Prevent uninitialized warnings mrezanin 2020-01-21 9:28 ` [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning mrezanin @ 2020-01-21 9:28 ` mrezanin 2020-01-21 10:02 ` Thomas Huth 2020-02-06 10:13 ` Laurent Vivier 1 sibling, 2 replies; 10+ messages in thread From: mrezanin @ 2020-01-21 9:28 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell From: Miroslav Rezanina <mrezanin@redhat.com> Compiler reports uninitialized warning for cmd_flags variable. Adding NULL initialization to prevent this warning. Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> --- hw/i2c/aspeed_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index 2da04a4..445182a 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -400,7 +400,7 @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus) { - g_autofree char *cmd_flags; + g_autofree char *cmd_flags = NULL; uint32_t count; if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning 2020-01-21 9:28 ` [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning mrezanin @ 2020-01-21 10:02 ` Thomas Huth 2020-01-21 10:44 ` Cédric Le Goater 2020-02-06 10:13 ` Laurent Vivier 1 sibling, 1 reply; 10+ messages in thread From: Thomas Huth @ 2020-01-21 10:02 UTC (permalink / raw) To: mrezanin, qemu-devel Cc: Andrew Jeffery, peter.maydell, qemu-trivial, Cédric Le Goater, Joel Stanley On 21/01/2020 10.28, mrezanin@redhat.com wrote: > From: Miroslav Rezanina <mrezanin@redhat.com> > > Compiler reports uninitialized warning for cmd_flags variable. > > Adding NULL initialization to prevent this warning. > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > --- > hw/i2c/aspeed_i2c.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > index 2da04a4..445182a 100644 > --- a/hw/i2c/aspeed_i2c.c > +++ b/hw/i2c/aspeed_i2c.c > @@ -400,7 +400,7 @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) > > static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus) > { > - g_autofree char *cmd_flags; > + g_autofree char *cmd_flags = NULL; > uint32_t count; > > if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) { Reviewed-by: Thomas Huth <thuth@redhat.com> ... maybe someone with enough Perl-foo (i.e. not me ;-)) should add a check to our check_patch.pl script so that it complains when new code is introduced that uses g_autofree without initializing the variable... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning 2020-01-21 10:02 ` Thomas Huth @ 2020-01-21 10:44 ` Cédric Le Goater 2020-01-21 10:57 ` Miroslav Rezanina 2020-01-21 11:43 ` Thomas Huth 0 siblings, 2 replies; 10+ messages in thread From: Cédric Le Goater @ 2020-01-21 10:44 UTC (permalink / raw) To: Thomas Huth, mrezanin, qemu-devel Cc: Andrew Jeffery, peter.maydell, qemu-trivial, Joel Stanley On 1/21/20 11:02 AM, Thomas Huth wrote: > On 21/01/2020 10.28, mrezanin@redhat.com wrote: >> From: Miroslav Rezanina <mrezanin@redhat.com> >> >> Compiler reports uninitialized warning for cmd_flags variable. >> >> Adding NULL initialization to prevent this warning. >> >> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> >> --- >> hw/i2c/aspeed_i2c.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c >> index 2da04a4..445182a 100644 >> --- a/hw/i2c/aspeed_i2c.c >> +++ b/hw/i2c/aspeed_i2c.c >> @@ -400,7 +400,7 @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) >> >> static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus) >> { >> - g_autofree char *cmd_flags; >> + g_autofree char *cmd_flags = NULL; >> uint32_t count; >> >> if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) { > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > ... maybe someone with enough Perl-foo (i.e. not me ;-)) should add a > check to our check_patch.pl script so that it complains when new code is > introduced that uses g_autofree without initializing the variable... weird. The cmd_flags variable is assigned just after and used in a trace. C. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning 2020-01-21 10:44 ` Cédric Le Goater @ 2020-01-21 10:57 ` Miroslav Rezanina 2020-01-21 11:43 ` Thomas Huth 1 sibling, 0 replies; 10+ messages in thread From: Miroslav Rezanina @ 2020-01-21 10:57 UTC (permalink / raw) To: Cédric Le Goater Cc: peter maydell, Thomas Huth, Andrew Jeffery, qemu-devel, qemu-trivial, Joel Stanley ----- Original Message ----- > From: "Cédric Le Goater" <clg@kaod.org> > To: "Thomas Huth" <thuth@redhat.com>, mrezanin@redhat.com, qemu-devel@nongnu.org > Cc: "peter maydell" <peter.maydell@linaro.org>, "Andrew Jeffery" <andrew@aj.id.au>, "Joel Stanley" <joel@jms.id.au>, > qemu-trivial@nongnu.org > Sent: Tuesday, January 21, 2020 11:44:14 AM > Subject: Re: [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning > > On 1/21/20 11:02 AM, Thomas Huth wrote: > > On 21/01/2020 10.28, mrezanin@redhat.com wrote: > >> From: Miroslav Rezanina <mrezanin@redhat.com> > >> > >> Compiler reports uninitialized warning for cmd_flags variable. > >> > >> Adding NULL initialization to prevent this warning. > >> > >> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > >> --- > >> hw/i2c/aspeed_i2c.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > >> index 2da04a4..445182a 100644 > >> --- a/hw/i2c/aspeed_i2c.c > >> +++ b/hw/i2c/aspeed_i2c.c > >> @@ -400,7 +400,7 @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) > >> > >> static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus) > >> { > >> - g_autofree char *cmd_flags; > >> + g_autofree char *cmd_flags = NULL; > >> uint32_t count; > >> > >> if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) { > > > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > > > ... maybe someone with enough Perl-foo (i.e. not me ;-)) should add a > > check to our check_patch.pl script so that it complains when new code is > > introduced that uses g_autofree without initializing the variable... > > weird. The cmd_flags variable is assigned just after and used > in a trace. > As g_autofree is used, variable has to be initialized otherwise will compiler complain even in the case we write to variable immediately after. Mirek > C. > > -- Miroslav Rezanina Software Engineer - Virtualization Team Maintainer ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning 2020-01-21 10:44 ` Cédric Le Goater 2020-01-21 10:57 ` Miroslav Rezanina @ 2020-01-21 11:43 ` Thomas Huth 1 sibling, 0 replies; 10+ messages in thread From: Thomas Huth @ 2020-01-21 11:43 UTC (permalink / raw) To: Cédric Le Goater, mrezanin, qemu-devel Cc: Andrew Jeffery, peter.maydell, qemu-trivial, Joel Stanley On 21/01/2020 11.44, Cédric Le Goater wrote: > On 1/21/20 11:02 AM, Thomas Huth wrote: >> On 21/01/2020 10.28, mrezanin@redhat.com wrote: >>> From: Miroslav Rezanina <mrezanin@redhat.com> >>> >>> Compiler reports uninitialized warning for cmd_flags variable. >>> >>> Adding NULL initialization to prevent this warning. >>> >>> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> >>> --- >>> hw/i2c/aspeed_i2c.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c >>> index 2da04a4..445182a 100644 >>> --- a/hw/i2c/aspeed_i2c.c >>> +++ b/hw/i2c/aspeed_i2c.c >>> @@ -400,7 +400,7 @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) >>> >>> static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus) >>> { >>> - g_autofree char *cmd_flags; >>> + g_autofree char *cmd_flags = NULL; >>> uint32_t count; >>> >>> if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) { >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> >> ... maybe someone with enough Perl-foo (i.e. not me ;-)) should add a >> check to our check_patch.pl script so that it complains when new code is >> introduced that uses g_autofree without initializing the variable... > > weird. The cmd_flags variable is assigned just after and used > in a trace. I don't know, but my guess is that you could compile with tracing disabled - in that case gcc might maybe optimize the assignment away, too... ? Thomas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning 2020-01-21 9:28 ` [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning mrezanin 2020-01-21 10:02 ` Thomas Huth @ 2020-02-06 10:13 ` Laurent Vivier 1 sibling, 0 replies; 10+ messages in thread From: Laurent Vivier @ 2020-02-06 10:13 UTC (permalink / raw) To: mrezanin, qemu-devel; +Cc: peter.maydell Le 21/01/2020 à 10:28, mrezanin@redhat.com a écrit : > From: Miroslav Rezanina <mrezanin@redhat.com> > > Compiler reports uninitialized warning for cmd_flags variable. > > Adding NULL initialization to prevent this warning. > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > --- > hw/i2c/aspeed_i2c.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > index 2da04a4..445182a 100644 > --- a/hw/i2c/aspeed_i2c.c > +++ b/hw/i2c/aspeed_i2c.c > @@ -400,7 +400,7 @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) > > static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus) > { > - g_autofree char *cmd_flags; > + g_autofree char *cmd_flags = NULL; > uint32_t count; > > if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) { > Applied to my trivial-patches branch. Thanks, Laurent ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-06 10:19 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-21 9:28 [PATCH 0/2] Prevent uninitialized warnings mrezanin 2020-01-21 9:28 ` [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning mrezanin 2020-01-21 9:58 ` Thomas Huth 2020-01-21 15:03 ` Robert Foley 2020-01-21 9:28 ` [PATCH 2/2] aspeed/i2c: Prevent uninitialized warning mrezanin 2020-01-21 10:02 ` Thomas Huth 2020-01-21 10:44 ` Cédric Le Goater 2020-01-21 10:57 ` Miroslav Rezanina 2020-01-21 11:43 ` Thomas Huth 2020-02-06 10:13 ` 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.