diff mbox series

[v2,4/5] plugins/loader: fix uninitialized variable warning in plugin_reset_uninstall()

Message ID 20201111142203.2359370-5-kuhn.chenqun@huawei.com
State New
Headers show
Series fix uninitialized variable warning | expand

Commit Message

Chen Qun Nov. 11, 2020, 2:22 p.m. UTC
After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
 that the statements in the macro must be executed. As a result, some variables
 assignment statements in the macro may be considered as unexecuted by the compiler.

When the -Wmaybe-uninitialized capability is enabled on GCC9,the compiler showed warning:
plugins/loader.c: In function ‘plugin_reset_uninstall’:
plugins/loader.c:382:15: warning: ‘ctx’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 382 |     data->ctx = ctx;
     |     ~~~~~~~~~~^~~~~

Add a default value for 'expire_time' to prevented the warning.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
---
Cc: "Alex Bennée" <alex.bennee@linaro.org>
---
 plugins/loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alex Bennée Nov. 11, 2020, 5:22 p.m. UTC | #1
Chen Qun <kuhn.chenqun@huawei.com> writes:

> After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
>  that the statements in the macro must be executed. As a result, some variables
>  assignment statements in the macro may be considered as unexecuted by the compiler.
>
> When the -Wmaybe-uninitialized capability is enabled on GCC9,the compiler showed warning:
> plugins/loader.c: In function ‘plugin_reset_uninstall’:
> plugins/loader.c:382:15: warning: ‘ctx’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>  382 |     data->ctx = ctx;
>      |     ~~~~~~~~~~^~~~~
>
> Add a default value for 'expire_time' to prevented the warning.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> ---
> Cc: "Alex Bennée" <alex.bennee@linaro.org>
> ---
>  plugins/loader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/plugins/loader.c b/plugins/loader.c
> index 8ac5dbc20f..88593fe138 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -367,7 +367,7 @@ void plugin_reset_uninstall(qemu_plugin_id_t id,
>                              bool reset)
>  {
>      struct qemu_plugin_reset_data *data;
> -    struct qemu_plugin_ctx *ctx;
> +    struct qemu_plugin_ctx *ctx = NULL;

This doesn't really fix anything because you would end up faulting if
you attempted to de-ref a NULL ctx. However...

>  
>      WITH_QEMU_LOCK_GUARD(&plugin.lock) {
>          ctx = plugin_id_to_ctx_locked(id);

...this can't fail. If the lookup failed and returned a NULL plugin then
we would abort(). So why can't the Euler Robot see that?
Chen Qun Nov. 13, 2020, 5:53 a.m. UTC | #2
> -----Original Message-----
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Sent: Thursday, November 12, 2020 1:23 AM
> To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>
> Cc: qemu-devel@nongnu.org; qemu-trivial@nongnu.org;
> peter.maydell@linaro.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>;
> ganqixin <ganqixin@huawei.com>; Euler Robot <euler.robot@huawei.com>
> Subject: Re: [PATCH v2 4/5] plugins/loader: fix uninitialized variable warning in
> plugin_reset_uninstall()
> 
> 
> Chen Qun <kuhn.chenqun@huawei.com> writes:
> 
> > After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot
> > identify  that the statements in the macro must be executed. As a
> > result, some variables  assignment statements in the macro may be
> considered as unexecuted by the compiler.
> >
> > When the -Wmaybe-uninitialized capability is enabled on GCC9,the compiler
> showed warning:
> > plugins/loader.c: In function ‘plugin_reset_uninstall’:
> > plugins/loader.c:382:15: warning: ‘ctx’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
> >  382 |     data->ctx = ctx;
> >      |     ~~~~~~~~~~^~~~~
> >
> > Add a default value for 'expire_time' to prevented the warning.
> >
> > Reported-by: Euler Robot <euler.robot@huawei.com>
> > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> > ---
> > Cc: "Alex Bennée" <alex.bennee@linaro.org>
> > ---
> >  plugins/loader.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/plugins/loader.c b/plugins/loader.c index
> > 8ac5dbc20f..88593fe138 100644
> > --- a/plugins/loader.c
> > +++ b/plugins/loader.c
> > @@ -367,7 +367,7 @@ void plugin_reset_uninstall(qemu_plugin_id_t id,
> >                              bool reset)  {
> >      struct qemu_plugin_reset_data *data;
> > -    struct qemu_plugin_ctx *ctx;
> > +    struct qemu_plugin_ctx *ctx = NULL;
> 
> This doesn't really fix anything because you would end up faulting if you
> attempted to de-ref a NULL ctx. However...
> 
> >
> >      WITH_QEMU_LOCK_GUARD(&plugin.lock) {
> >          ctx = plugin_id_to_ctx_locked(id);
> 
> ...this can't fail. If the lookup failed and returned a NULL plugin then we would
> abort(). So why can't the Euler Robot see that?
>
Hi Alex ,
  As the commit message says, this warning is reported by GCC 9.3 compilation.
EulerRobot configures various compilation options and uses GCC or Clang to compilation tests.

This warning has occurred since WITH_QEMU_LOCK_GUARD was added, because the current compiler thinks that the statements in WITH_QEMU_LOCK_GUARD{ } may not be executed successfully.
In fact, it may be wrong, but the current compiler is not very smart. So, this patch only adds an initialization, if it does not have a bad effect, it can fix the warning which may exist for a long time.

Thanks,
Chen Qun
diff mbox series

Patch

diff --git a/plugins/loader.c b/plugins/loader.c
index 8ac5dbc20f..88593fe138 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -367,7 +367,7 @@  void plugin_reset_uninstall(qemu_plugin_id_t id,
                             bool reset)
 {
     struct qemu_plugin_reset_data *data;
-    struct qemu_plugin_ctx *ctx;
+    struct qemu_plugin_ctx *ctx = NULL;
 
     WITH_QEMU_LOCK_GUARD(&plugin.lock) {
         ctx = plugin_id_to_ctx_locked(id);