Re: [PATCH 1/2] qga/vss-win32: Fix ConvertStringToBSTR redefinition with newer MinGW

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 12/16/25 1:53 AM, Kostiantyn Kostiuk wrote:
> Hi
> 
> On Tue, Dec 16, 2025 at 8:26 AM Marc-André Lureau 
> <marcandre.lureau@redhat.com <mailto:marcandre.lureau@redhat.com>> wrote:
> 
>     Hi
> 
>     On Mon, Dec 15, 2025 at 8:57 PM <phind.uet@gmail.com
>     <mailto:phind.uet@gmail.com>> wrote:
> 
>         From: Nguyen Dinh Phi <phind.uet@gmail.com
>         <mailto:phind.uet@gmail.com>>
> 
>         Newer versions of MinGW-w64 provide ConvertStringToBSTR() in the
>         _com_util namespace via <comutil.h>. This causes a redefinition
>         error when building qemu-ga on Windows with these toolchains.
> 
>         Add a meson check to detect whether ConvertStringToBSTR is already
>         available, and conditionally compile our fallback implementation
>         only when the system does not provide one.
> 
>         Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com
>         <mailto:phind.uet@gmail.com>>
>         ---
>           meson.build               | 12 ++++++++++++
>           qga/vss-win32/install.cpp |  2 ++
>           2 files changed, 14 insertions(+)
> 
>         diff --git a/meson.build b/meson.build
>         index c5710a6a47..60a980e610 100644
>         --- a/meson.build
>         +++ b/meson.build
>         @@ -3299,6 +3299,18 @@ endif
>           # Detect host pointer size for the target configuration loop.
>           host_long_bits = cc.sizeof('void *') * 8
> 
>         +# Detect if ConvertStringToBSTR has been defined in _com_util
>         namespace
>         +if host_os == 'windows'
>         +  has_convert_string_to_bstr = cxx.compiles('''
>         +    #include <comutil.h>
>         +    int main() {
>         +        BSTR b = _com_util::ConvertStringToBSTR("test");
>         +        return b ? 0 : 1;
>         +    }
>         +  ''')
>         +  config_host_data.set('CONFIG_CONVERT_STRING_TO_BSTR',
>         has_convert_string_to_bstr)
>         +endif
>         +
>           ########################
>           # Target configuration #
>           ########################
>         diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
>         index 7b25d9098b..5b7a8e9bc5 100644
>         --- a/qga/vss-win32/install.cpp
>         +++ b/qga/vss-win32/install.cpp
>         @@ -549,6 +549,7 @@ STDAPI DllUnregisterServer(void)
> 
> 
>           /* Support function to convert ASCII string into BSTR (used in
>         _bstr_t) */
>         +#ifndef CONFIG_CONVERT_STRING_TO_BSTR
> 
> 
>     I wonder if it could check __MINGW64_VERSION_MAJOR >= 14 instead of
>     adding a configure-time check.
> 
> 
> @Peter Maydell <mailto:peter.maydell@linaro.org> preferred to avoid 
> specific version-number checks.
> See: https://gitlab.com/qemu-project/qemu/-/issues/3217#note_2935451782 
> <https://gitlab.com/qemu-project/qemu/-/issues/3217#note_2935451782>
> 
> I also preferred the idea of checking the real function present instead 
> of the version of the component.
> 
> Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com 
> <mailto:kkostiuk@redhat.com>>
> 
> 
>     lgtm anyway
>     Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
>     <mailto:marcandre.lureau@redhat.com>>
> 
>           namespace _com_util
>           {
>               BSTR WINAPI ConvertStringToBSTR(const char *ascii) {
>         @@ -566,6 +567,7 @@ namespace _com_util
>                   return bstr;
>               }
>           }
>         +#endif
> 
>           /* Stop QGA VSS provider service using Winsvc API  */
>           STDAPI StopService(void)
>         -- 
>         2.43.0
> 

When cross-compiling using fedora-win64-cross, it can find the function 
in header but does not link it.

$ podman run --pull newer --init --rm -it -v $(pwd):$(pwd) -w $(pwd) \
docker.io/pbolinaro/qemu-ci:fedora-win64-cross \
bash -cx './configure $QEMU_CONFIGURE_OPTS && ninja -C build install'
...
[4434/4696] Linking target qga/vss-win32/qga-vss.dll
FAILED: qga/vss-win32/qga-vss.dll
x86_64-w64-mingw32-g++ -m64  -o qga/vss-win32/qga-vss.dll 
qga/vss-win32/qga-vss.dll.p/requester.cpp.obj 
qga/vss-win32/qga-vss.dll.p/provider.cpp.obj 
qga/vss-win32/qga-vss.dll.p/install.cpp.obj 
qga/vss-win32/qga-vss.dll.p/vss-debug.cpp.obj 
-Wl,--allow-shlib-undefined -shared ../qga/vss-win32/qga-vss.def 
-Wl,--start-group -Wl,--out-implib=qga/vss-win32/qga-vss.dll.a 
-fstack-protector-strong -Wl,--no-seh -Wl,--nxcompat -Wl,--dynamicbase 
-Wl,--high-entropy-va -fstack-protector-all -fstack-protector-strong 
-Wl,--add-stdcall-alias -Wl,--enable-stdcall-fixup -lws2_32 -lole32 
-loleaut32 -lshlwapi -luuid -lkernel32 -luser32 -lgdi32 -lwinspool 
-lshell32 -lcomdlg32 -ladvapi32 -Wl,--end-group
/usr/lib/gcc/x86_64-w64-mingw32/14.2.1/../../../../x86_64-w64-mingw32/bin/ld: 
qga/vss-win32/qga-vss.dll.p/install.cpp.obj: in function 
`_bstr_t::Data_t::Data_t(char const*)':
/usr/x86_64-w64-mingw32/sys-root/mingw/include/comutil.h:279:(.text+0x1dc8): 
undefined reference to `_com_util::ConvertStringToBSTR(char const*)'
...

Looking into mingw files, the symbol is not there.
$ nm /usr/x86_64-w64-mingw32/sys-root/mingw/lib/lib*.a |
grep -i ConvertStringToBSTR

It's present in header file though:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/comutil.h

Not sure if something is missing in mingw, or if it's missing an 
attribute to say it should be available at runtime only, but in all 
cases, a simple fix to the patch is:

diff --git a/meson.build b/meson.build
index ab19317f5af..113296544c4 100644
--- a/meson.build
+++ b/meson.build
@@ -3261,7 +3261,7 @@ host_long_bits = cc.sizeof('void *') * 8

  # Detect if ConvertStringToBSTR has been defined in _com_util namespace
  if host_os == 'windows'
-  has_convert_string_to_bstr = cxx.compiles('''
+  has_convert_string_to_bstr = cxx.links('''
      #include <comutil.h>
      int main() {
          BSTR b = _com_util::ConvertStringToBSTR("test");

I would recommend to use cxx.links instead of cxx.compiles for now.

Regards,
Pierrick