[PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning

mrezanin@redhat.com posted 2 patches 6 years ago
Maintainers: Joel Stanley <joel@jms.id.au>, "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@aj.id.au>
[PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning
Posted by mrezanin@redhat.com 6 years ago
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


Re: [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning
Posted by Thomas Huth 6 years ago
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>


Re: [PATCH 1/2] test-logging: Fix -Werror=maybe-uninitialized warning
Posted by Robert Foley 6 years ago
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>
>