[PATCH v4 3/4] qga/vss-win32: fix warning for clang++-15

Pierrick Bouvier posted 4 patches 2 years, 11 months ago
Maintainers: Stefan Weil <sw@weilnetz.de>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Konstantin Kostiuk <kkostiuk@redhat.com>, Michael Roth <michael.roth@amd.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Richard Henderson <richard.henderson@linaro.org>
[PATCH v4 3/4] qga/vss-win32: fix warning for clang++-15
Posted by Pierrick Bouvier 2 years, 11 months ago
Reported when compiling with clang-windows-arm64.

../qga/vss-win32/install.cpp:537:9: error: variable 'hr' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
    if (!(ControlService(service, SERVICE_CONTROL_STOP, NULL))) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../qga/vss-win32/install.cpp:545:12: note: uninitialized use occurs here
    return hr;
           ^~
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Fixes: 917ebcb170 ("qga-win: Fix QGA VSS Provider service stop failure")
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 qga/vss-win32/install.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index b57508fbe0..b8087e5baa 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -518,7 +518,7 @@ namespace _com_util
 /* Stop QGA VSS provider service using Winsvc API  */
 STDAPI StopService(void)
 {
-    HRESULT hr;
+    HRESULT hr = S_OK;
     SC_HANDLE manager = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS);
     SC_HANDLE service = NULL;
 
-- 
2.30.2


Re: [PATCH v4 3/4] qga/vss-win32: fix warning for clang++-15
Posted by Pierrick Bouvier 2 years, 10 months ago
Sorry to come back on this, but it seems this specific commit was not 
integrated in trunk.

@Konstantin Kostiuk: If you plan to integrate this later (before 8.0 
tag), sorry for the noise. Since rc1 was published today, I think it may 
have been "lost".

If someone wants to merge it, that would be nice.

Thanks,
Pierrick

On 2/21/23 16:30, Pierrick Bouvier wrote:
> Reported when compiling with clang-windows-arm64.
> 
> ../qga/vss-win32/install.cpp:537:9: error: variable 'hr' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>      if (!(ControlService(service, SERVICE_CONTROL_STOP, NULL))) {
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../qga/vss-win32/install.cpp:545:12: note: uninitialized use occurs here
>      return hr;
>             ^~
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Fixes: 917ebcb170 ("qga-win: Fix QGA VSS Provider service stop failure")
> Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   qga/vss-win32/install.cpp | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
> index b57508fbe0..b8087e5baa 100644
> --- a/qga/vss-win32/install.cpp
> +++ b/qga/vss-win32/install.cpp
> @@ -518,7 +518,7 @@ namespace _com_util
>   /* Stop QGA VSS provider service using Winsvc API  */
>   STDAPI StopService(void)
>   {
> -    HRESULT hr;
> +    HRESULT hr = S_OK;
>       SC_HANDLE manager = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS);
>       SC_HANDLE service = NULL;
>   
Re: [PATCH v4 3/4] qga/vss-win32: fix warning for clang++-15
Posted by Konstantin Kostiuk 2 years, 10 months ago
Hi Pierrick,

Thanks for reminding me. You are fully right to ping me. I really lost this
commit.
As QEMU is already at the code freeze stage, I don't want to push this into
8.0.
I hope it will be ok to merge after 8.0 was released.

Best Regards,
Konstantin Kostiuk.


On Tue, Mar 21, 2023 at 11:48 PM Pierrick Bouvier <
pierrick.bouvier@linaro.org> wrote:

> Sorry to come back on this, but it seems this specific commit was not
> integrated in trunk.
>
> @Konstantin Kostiuk: If you plan to integrate this later (before 8.0
> tag), sorry for the noise. Since rc1 was published today, I think it may
> have been "lost".
>
> If someone wants to merge it, that would be nice.
>
> Thanks,
> Pierrick
>
> On 2/21/23 16:30, Pierrick Bouvier wrote:
> > Reported when compiling with clang-windows-arm64.
> >
> > ../qga/vss-win32/install.cpp:537:9: error: variable 'hr' is used
> uninitialized whenever 'if' condition is false
> [-Werror,-Wsometimes-uninitialized]
> >      if (!(ControlService(service, SERVICE_CONTROL_STOP, NULL))) {
> >          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ../qga/vss-win32/install.cpp:545:12: note: uninitialized use occurs here
> >      return hr;
> >             ^~
> > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > Fixes: 917ebcb170 ("qga-win: Fix QGA VSS Provider service stop failure")
> > Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   qga/vss-win32/install.cpp | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
> > index b57508fbe0..b8087e5baa 100644
> > --- a/qga/vss-win32/install.cpp
> > +++ b/qga/vss-win32/install.cpp
> > @@ -518,7 +518,7 @@ namespace _com_util
> >   /* Stop QGA VSS provider service using Winsvc API  */
> >   STDAPI StopService(void)
> >   {
> > -    HRESULT hr;
> > +    HRESULT hr = S_OK;
> >       SC_HANDLE manager = OpenSCManager(NULL, NULL,
> SC_MANAGER_ALL_ACCESS);
> >       SC_HANDLE service = NULL;
> >
>
Re: [PATCH v4 3/4] qga/vss-win32: fix warning for clang++-15
Posted by Daniel P. Berrangé 2 years, 10 months ago
On Wed, Mar 22, 2023 at 07:18:11PM +0200, Konstantin Kostiuk wrote:
> Hi Pierrick,
> 
> Thanks for reminding me. You are fully right to ping me. I really lost this
> commit.
> As QEMU is already at the code freeze stage, I don't want to push this into
> 8.0.

FWIW, this kind of fix is perfectly ok to merge during code freeze,
especially in rc0/rc1 timeframe. The initial freeze date merely cuts
off new feature additions, but bug fixes are still allowed. Once we
get to rc2 then we're more focused on bug fixes that target regressions
from the previous release.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|