* [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
* [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 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 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 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
* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).