[PATCH] qga/vss-win32/install: Remove _com_util::ConvertStringToBSTR()

Bernhard Beschow posted 1 patch 3 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251101130330.1927-1-shentey@gmail.com
Maintainers: Kostiantyn Kostiuk <kkostiuk@redhat.com>, Michael Roth <michael.roth@amd.com>
qga/vss-win32/install.cpp | 19 -------------------
1 file changed, 19 deletions(-)
[PATCH] qga/vss-win32/install: Remove _com_util::ConvertStringToBSTR()
Posted by Bernhard Beschow 3 months, 1 week ago
Now that MSYS2 provides an implementation of the function it clashes with
QEMU's, resulting in a compilation error. Remove it since it doesn't seem
to be used anyway.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 qga/vss-win32/install.cpp | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index 7b25d9098b..147bf387fd 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -548,25 +548,6 @@ STDAPI DllUnregisterServer(void)
 }
 
 
-/* Support function to convert ASCII string into BSTR (used in _bstr_t) */
-namespace _com_util
-{
-    BSTR WINAPI ConvertStringToBSTR(const char *ascii) {
-        int len = strlen(ascii);
-        BSTR bstr = SysAllocStringLen(NULL, len);
-
-        if (!bstr) {
-            return NULL;
-        }
-
-        if (mbstowcs(bstr, ascii, len) == (size_t)-1) {
-            qga_debug("Failed to convert string '%s' into BSTR", ascii);
-            bstr[0] = 0;
-        }
-        return bstr;
-    }
-}
-
 /* Stop QGA VSS provider service using Winsvc API  */
 STDAPI StopService(void)
 {
-- 
2.51.2
Re: [PATCH] qga/vss-win32/install: Remove _com_util::ConvertStringToBSTR()
Posted by Peter Maydell 3 months, 1 week ago
On Sat, 1 Nov 2025 at 13:04, Bernhard Beschow <shentey@gmail.com> wrote:
>
> Now that MSYS2 provides an implementation of the function it clashes with
> QEMU's, resulting in a compilation error. Remove it since it doesn't seem
> to be used anyway.

The comment says it's used by _bstr_t, which presumably
is in some Windows header or library that we're linking against.
Our code seems to use _bstr_t a lot. Is this function definitely
not required, or should we have something so we provide it only
when MSYS2 does not?

thanks
-- PMM
Re: [PATCH] qga/vss-win32/install: Remove _com_util::ConvertStringToBSTR()
Posted by Kostiantyn Kostiuk 3 months, 1 week ago
This function is used inside Windows headers. You can find the following
code in comutil.h in MinGW

inline _bstr_t::Data_t::Data_t(const char *s) : m_str(NULL),m_RefCount(1) {
m_wstr = _com_util::ConvertStringToBSTR(s);
}

But MinGW does not implement ConvertStringToBSTR, so QEMU should implement
this function.
We use _bstr_t in VSS-provided DLL, so this function is mandatory for us.


Best Regards,
Kostiantyn Kostiuk.


On Sat, Nov 1, 2025 at 3:13 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Sat, 1 Nov 2025 at 13:04, Bernhard Beschow <shentey@gmail.com> wrote:
> >
> > Now that MSYS2 provides an implementation of the function it clashes with
> > QEMU's, resulting in a compilation error. Remove it since it doesn't seem
> > to be used anyway.
>
> The comment says it's used by _bstr_t, which presumably
> is in some Windows header or library that we're linking against.
> Our code seems to use _bstr_t a lot. Is this function definitely
> not required, or should we have something so we provide it only
> when MSYS2 does not?
>
> thanks
> -- PMM
>
>
Re: [PATCH] qga/vss-win32/install: Remove _com_util::ConvertStringToBSTR()
Posted by Kostiantyn Kostiuk 3 months, 1 week ago
CC @Yan Vugenfirer <yvugenfi@redhat.com>

Best Regards,
Kostiantyn Kostiuk.


On Mon, Nov 3, 2025 at 9:30 AM Kostiantyn Kostiuk <kkostiuk@redhat.com>
wrote:

>
> This function is used inside Windows headers. You can find the following
> code in comutil.h in MinGW
>
> inline _bstr_t::Data_t::Data_t(const char *s) : m_str(NULL),m_RefCount(1)
> {
> m_wstr = _com_util::ConvertStringToBSTR(s);
> }
>
> But MinGW does not implement ConvertStringToBSTR, so QEMU should implement
> this function.
> We use _bstr_t in VSS-provided DLL, so this function is mandatory for us.
>
>
> Best Regards,
> Kostiantyn Kostiuk.
>
>
> On Sat, Nov 1, 2025 at 3:13 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On Sat, 1 Nov 2025 at 13:04, Bernhard Beschow <shentey@gmail.com> wrote:
>> >
>> > Now that MSYS2 provides an implementation of the function it clashes
>> with
>> > QEMU's, resulting in a compilation error. Remove it since it doesn't
>> seem
>> > to be used anyway.
>>
>> The comment says it's used by _bstr_t, which presumably
>> is in some Windows header or library that we're linking against.
>> Our code seems to use _bstr_t a lot. Is this function definitely
>> not required, or should we have something so we provide it only
>> when MSYS2 does not?
>>
>> thanks
>> -- PMM
>>
>>
Re: [PATCH] qga/vss-win32/install: Remove _com_util::ConvertStringToBSTR()
Posted by Bernhard Beschow 3 months, 1 week ago

Am 1. November 2025 13:13:26 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Sat, 1 Nov 2025 at 13:04, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Now that MSYS2 provides an implementation of the function it clashes with
>> QEMU's, resulting in a compilation error. Remove it since it doesn't seem
>> to be used anyway.
>
>The comment says it's used by _bstr_t, which presumably
>is in some Windows header or library that we're linking against.
>Our code seems to use _bstr_t a lot. Is this function definitely
>not required, or should we have something so we provide it only
>when MSYS2 does not?

I just grepped the code and it doesn't return any users. And searching the git history it never did. Maybe some library we're linking against needs an implementation but I can't reproduce this with my recent MSYS2 installation.

Best regards,
Bernhard

>
>thanks
>-- PMM
Re: [PATCH] qga/vss-win32/install: Remove _com_util::ConvertStringToBSTR()
Posted by Peter Maydell 3 months, 1 week ago
On Sat, 1 Nov 2025 at 13:48, Bernhard Beschow <shentey@gmail.com> wrote:
>
>
>
> Am 1. November 2025 13:13:26 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
> >On Sat, 1 Nov 2025 at 13:04, Bernhard Beschow <shentey@gmail.com> wrote:
> >>
> >> Now that MSYS2 provides an implementation of the function it clashes with
> >> QEMU's, resulting in a compilation error. Remove it since it doesn't seem
> >> to be used anyway.
> >
> >The comment says it's used by _bstr_t, which presumably
> >is in some Windows header or library that we're linking against.
> >Our code seems to use _bstr_t a lot. Is this function definitely
> >not required, or should we have something so we provide it only
> >when MSYS2 does not?
>
> I just grepped the code and it doesn't return any users. And searching
> the git history it never did.

The comment suggests that the user would be in the MS headers
we're including or the library we're linking against, not in our
own code, though, so grep wouldn't find it.

-- PMM