qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] util: win32: Write hex value when can't get error message
@ 2025-07-17 14:59 Kostiantyn Kostiuk
  2025-07-17 15:28 ` Yan Vugenfirer
  2025-07-19  6:27 ` Markus Armbruster
  0 siblings, 2 replies; 12+ messages in thread
From: Kostiantyn Kostiuk @ 2025-07-17 14:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Yan Vugenfirer, Daniel Berrangé,
	Kostiantyn Kostiuk

g_win32_error_message - translate a Win32 error code
(as returned by GetLastError()) into the corresponding message.

In the same time, we call error_setg_win32_internal with
error codes from different Windows componets like VSS or
Performance monitor that provides different codes and
can't be converted with g_win32_error_message. In this
case, the empty suffix will be returned so error will be
masked.

QGA error example:
 - before changes:
  {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: "}}
 - after changes:
  {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: unknown Windows error 0x8004230e"}}

Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
---
 util/error.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/util/error.c b/util/error.c
index daea2142f3..b1342558ae 100644
--- a/util/error.c
+++ b/util/error.c
@@ -188,6 +188,11 @@ void error_setg_win32_internal(Error **errp,
 
     if (win32_err != 0) {
         suffix = g_win32_error_message(win32_err);
+        // g_win32_error_message() failed
+        if (!suffix[0]) {
+            g_free(suffix);
+            suffix = g_strdup_printf("unknown Windows error 0x%x", win32_err);
+        }
     }
 
     va_start(ap, fmt);
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-17 14:59 [PATCH] util: win32: Write hex value when can't get error message Kostiantyn Kostiuk
@ 2025-07-17 15:28 ` Yan Vugenfirer
  2025-07-19  6:27 ` Markus Armbruster
  1 sibling, 0 replies; 12+ messages in thread
From: Yan Vugenfirer @ 2025-07-17 15:28 UTC (permalink / raw)
  To: Kostiantyn Kostiuk; +Cc: qemu-devel, Markus Armbruster, Daniel Berrangé

On Thu, Jul 17, 2025 at 5:59 PM Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>
> g_win32_error_message - translate a Win32 error code
> (as returned by GetLastError()) into the corresponding message.
>
> In the same time, we call error_setg_win32_internal with
> error codes from different Windows componets like VSS or
> Performance monitor that provides different codes and
> can't be converted with g_win32_error_message. In this
> case, the empty suffix will be returned so error will be
> masked.
>
> QGA error example:
>  - before changes:
>   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: "}}
>  - after changes:
>   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: unknown Windows error 0x8004230e"}}
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> ---
>  util/error.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/util/error.c b/util/error.c
> index daea2142f3..b1342558ae 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -188,6 +188,11 @@ void error_setg_win32_internal(Error **errp,
>
>      if (win32_err != 0) {
>          suffix = g_win32_error_message(win32_err);
> +        // g_win32_error_message() failed
> +        if (!suffix[0]) {
> +            g_free(suffix);
> +            suffix = g_strdup_printf("unknown Windows error 0x%x", win32_err);
> +        }
>      }
>
>      va_start(ap, fmt);
> --
> 2.48.1
>

Reviewed-by: Yan Vugenfirer <yvugenfi@redhat.com>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-17 14:59 [PATCH] util: win32: Write hex value when can't get error message Kostiantyn Kostiuk
  2025-07-17 15:28 ` Yan Vugenfirer
@ 2025-07-19  6:27 ` Markus Armbruster
  2025-07-21  7:07   ` Kostiantyn Kostiuk
  1 sibling, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2025-07-19  6:27 UTC (permalink / raw)
  To: Kostiantyn Kostiuk; +Cc: qemu-devel, Yan Vugenfirer, Daniel Berrangé

Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:

> g_win32_error_message - translate a Win32 error code
> (as returned by GetLastError()) into the corresponding message.
>
> In the same time, we call error_setg_win32_internal with
> error codes from different Windows componets like VSS or
> Performance monitor that provides different codes and
> can't be converted with g_win32_error_message.

Are these error codes from GetLastError()?

>                                                In this
> case, the empty suffix will be returned so error will be
> masked.
>
> QGA error example:
>  - before changes:
>   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: "}}
>  - after changes:
>   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: unknown Windows error 0x8004230e"}}

Exact reproducer?

> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> ---
>  util/error.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/util/error.c b/util/error.c
> index daea2142f3..b1342558ae 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -188,6 +188,11 @@ void error_setg_win32_internal(Error **errp,
>  
>      if (win32_err != 0) {
>          suffix = g_win32_error_message(win32_err);
> +        // g_win32_error_message() failed
> +        if (!suffix[0]) {
> +            g_free(suffix);
> +            suffix = g_strdup_printf("unknown Windows error 0x%x", win32_err);
> +        }
>      }
>  
>      va_start(ap, fmt);



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-19  6:27 ` Markus Armbruster
@ 2025-07-21  7:07   ` Kostiantyn Kostiuk
  2025-07-21  9:22     ` Markus Armbruster
  0 siblings, 1 reply; 12+ messages in thread
From: Kostiantyn Kostiuk @ 2025-07-21  7:07 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, Yan Vugenfirer, Daniel Berrangé

[-- Attachment #1: Type: text/plain, Size: 2592 bytes --]

On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <armbru@redhat.com> wrote:

> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>
> > g_win32_error_message - translate a Win32 error code
> > (as returned by GetLastError()) into the corresponding message.
> >
> > In the same time, we call error_setg_win32_internal with
> > error codes from different Windows componets like VSS or
> > Performance monitor that provides different codes and
> > can't be converted with g_win32_error_message.
>
> Are these error codes from GetLastError()?
>

No.
VSS functions directly return an error code.
Section: Return value -
https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset

Performance Counters API can return a system error code or a PDH error code.
Section: Return value -
https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhopenqueryw
System error code = GetLastError, PDH error code, something else.

https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-error-codes
FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.


> >                                                In this
> > case, the empty suffix will be returned so error will be
> > masked.
> >
> > QGA error example:
> >  - before changes:
> >   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to
> snapshot set: "}}
> >  - after changes:
> >   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to
> snapshot set: unknown Windows error 0x8004230e"}}
>
> Exact reproducer?
>

Yes, for example, with VSS.
Execute: {"execute": "guest-fsfreeze-freeze-list", "arguments":
{"mountpoints": ["C:"]}}
After this commit, you will get {"error": {"class": "GenericError", "desc":
"failed to add C: to snapshot set: unknown Windows error 0x80042308"}}


>
> > Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> > ---
> >  util/error.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/util/error.c b/util/error.c
> > index daea2142f3..b1342558ae 100644
> > --- a/util/error.c
> > +++ b/util/error.c
> > @@ -188,6 +188,11 @@ void error_setg_win32_internal(Error **errp,
> >
> >      if (win32_err != 0) {
> >          suffix = g_win32_error_message(win32_err);
> > +        // g_win32_error_message() failed
> > +        if (!suffix[0]) {
> > +            g_free(suffix);
> > +            suffix = g_strdup_printf("unknown Windows error 0x%x",
> win32_err);
> > +        }
> >      }
> >
> >      va_start(ap, fmt);
>
>

[-- Attachment #2: Type: text/html, Size: 4524 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-21  7:07   ` Kostiantyn Kostiuk
@ 2025-07-21  9:22     ` Markus Armbruster
  2025-07-21 10:12       ` Kostiantyn Kostiuk
  0 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2025-07-21  9:22 UTC (permalink / raw)
  To: Kostiantyn Kostiuk; +Cc: qemu-devel, Yan Vugenfirer, Daniel Berrangé

Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:

> On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>>
>> > g_win32_error_message - translate a Win32 error code
>> > (as returned by GetLastError()) into the corresponding message.
>> >
>> > In the same time, we call error_setg_win32_internal with
>> > error codes from different Windows componets like VSS or
>> > Performance monitor that provides different codes and
>> > can't be converted with g_win32_error_message.
>>
>> Are these error codes from GetLastError()?
>>
>
> No.
> VSS functions directly return an error code.
> Section: Return value -
> https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
>
> Performance Counters API can return a system error code or a PDH error code.
> Section: Return value -
> https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhopenqueryw
> System error code = GetLastError, PDH error code, something else.
>
> https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-error-codes
> FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.

The error code error_setg_win32() takes is passed to
g_win32_error_message().  Contract:

    g_win32_error_message ()

    gchar *
    g_win32_error_message (gint error);

    Translate a Win32 error code (as returned by GetLastError() or
    WSAGetLastError()) into the corresponding message.  The message is
    either language neutral, or in the thread's language, or the user's
    language, the system's language, or US English (see docs for
    FormatMessage()).  The returned string is in UTF-8.  It should be
    deallocated with g_free().

    Parameters

        error error code.

    Returns

        newly-allocated error message

https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-Functions.php#g-win32-error-message

Passing error codes from sources other than GetLastError() or
WSAGetLastError() violates this contract.

Apparently, g_win32_error_message() returns NULL then.  This is not
documented behavior.

Your fix relies on this undocumented behavior.

I believe we should instead fix the misuses of error_setg_win32().

[...]



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-21  9:22     ` Markus Armbruster
@ 2025-07-21 10:12       ` Kostiantyn Kostiuk
  2025-07-21 11:23         ` Yan Vugenfirer
  2025-07-21 11:53         ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 12+ messages in thread
From: Kostiantyn Kostiuk @ 2025-07-21 10:12 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, Yan Vugenfirer, Daniel Berrangé

[-- Attachment #1: Type: text/plain, Size: 3631 bytes --]

On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <armbru@redhat.com>
wrote:

> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>
> > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
> >>
> >> > g_win32_error_message - translate a Win32 error code
> >> > (as returned by GetLastError()) into the corresponding message.
> >> >
> >> > In the same time, we call error_setg_win32_internal with
> >> > error codes from different Windows componets like VSS or
> >> > Performance monitor that provides different codes and
> >> > can't be converted with g_win32_error_message.
> >>
> >> Are these error codes from GetLastError()?
> >>
> >
> > No.
> > VSS functions directly return an error code.
> > Section: Return value -
> >
> https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
> >
> > Performance Counters API can return a system error code or a PDH error
> code.
> > Section: Return value -
> >
> https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhopenqueryw
> > System error code = GetLastError, PDH error code, something else.
> >
> > https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-error-codes
> > FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.
>
> The error code error_setg_win32() takes is passed to
> g_win32_error_message().  Contract:
>
>     g_win32_error_message ()
>
>     gchar *
>     g_win32_error_message (gint error);
>
>     Translate a Win32 error code (as returned by GetLastError() or
>     WSAGetLastError()) into the corresponding message.  The message is
>     either language neutral, or in the thread's language, or the user's
>     language, the system's language, or US English (see docs for
>     FormatMessage()).  The returned string is in UTF-8.  It should be
>     deallocated with g_free().
>
>     Parameters
>
>         error error code.
>
>     Returns
>
>         newly-allocated error message
>
>
> https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-Functions.php#g-win32-error-message
>
> Passing error codes from sources other than GetLastError() or
> WSAGetLastError() violates this contract.
>
> Apparently, g_win32_error_message() returns NULL then.  This is not
> documented behavior.
>

It returns an empty string, not NULL.
https://gitlab.gnome.org/GNOME/glib/-/blob/a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216


> Your fix relies on this undocumented behavior.
>

As for me, this behaviour is almost expected (I expected NULL instead of an
empty string) because
g_win32_error_message uses FormatMessage, and FormatMessage returns NULL if
an unknown error code
is provided. And I know this, because FormatMessage is the only way on
Windows to get a human-readable
string from the error code.


>
> I believe we should instead fix the misuses of error_setg_win32().
>

This will be more complicated.
1. I don't know what code was returned by the Performance Counters API
(system or PDH)
2. QGA call error_setg_win32_internal as part of error propagation with
different error codes,
 fix the misuses in this case means have a different error propagator for
different error codes (back to 1)
3. Also, this means that I need to reimplement error_setg_win32_internal
for non-system errors with only
one difference: "unknown Windows error 0x%x" instead of
g_win32_error_message, because anyway
I need the full error propagation part.


> [...]
>
>

[-- Attachment #2: Type: text/html, Size: 5755 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-21 10:12       ` Kostiantyn Kostiuk
@ 2025-07-21 11:23         ` Yan Vugenfirer
  2025-07-21 11:53         ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 12+ messages in thread
From: Yan Vugenfirer @ 2025-07-21 11:23 UTC (permalink / raw)
  To: Kostiantyn Kostiuk; +Cc: Markus Armbruster, qemu-devel, Daniel Berrangé

On Mon, Jul 21, 2025 at 1:12 PM Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>
>
>
> On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>>
>> > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> >> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>> >>
>> >> > g_win32_error_message - translate a Win32 error code
>> >> > (as returned by GetLastError()) into the corresponding message.
>> >> >
>> >> > In the same time, we call error_setg_win32_internal with
>> >> > error codes from different Windows componets like VSS or
>> >> > Performance monitor that provides different codes and
>> >> > can't be converted with g_win32_error_message.
>> >>
>> >> Are these error codes from GetLastError()?
>> >>
>> >
>> > No.
>> > VSS functions directly return an error code.
>> > Section: Return value -
>> > https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
>> >
>> > Performance Counters API can return a system error code or a PDH error code.
>> > Section: Return value -
>> > https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhopenqueryw
>> > System error code = GetLastError, PDH error code, something else.
>> >
>> > https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-error-codes
>> > FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.
>>
>> The error code error_setg_win32() takes is passed to
>> g_win32_error_message().  Contract:
>>
>>     g_win32_error_message ()
>>
>>     gchar *
>>     g_win32_error_message (gint error);
>>
>>     Translate a Win32 error code (as returned by GetLastError() or
>>     WSAGetLastError()) into the corresponding message.  The message is
>>     either language neutral, or in the thread's language, or the user's
>>     language, the system's language, or US English (see docs for
>>     FormatMessage()).  The returned string is in UTF-8.  It should be
>>     deallocated with g_free().
>>
>>     Parameters
>>
>>         error error code.
>>
>>     Returns
>>
>>         newly-allocated error message
>>
>> https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-Functions.php#g-win32-error-message
>>
>> Passing error codes from sources other than GetLastError() or
>> WSAGetLastError() violates this contract.
>>
>> Apparently, g_win32_error_message() returns NULL then.  This is not
>> documented behavior.

If glib wrapper behaviour is undocumented, let's not use it in Windows
related code and use Win32 API directly.
We can also fix the documentation.

Also, if we take a look at
https://learn.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror
then it looks like GetLastError might return error codes that don't
have strings in the system, again this is exactly the case were we
want the numeric value to be propagated and not silently dropped.

Best regards,
Yan.

>
>
> It returns an empty string, not NULL.
> https://gitlab.gnome.org/GNOME/glib/-/blob/a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216
>
>>
>> Your fix relies on this undocumented behavior.
>
>
> As for me, this behaviour is almost expected (I expected NULL instead of an empty string) because
> g_win32_error_message uses FormatMessage, and FormatMessage returns NULL if an unknown error code
> is provided. And I know this, because FormatMessage is the only way on Windows to get a human-readable
> string from the error code.
>
>>
>>
>> I believe we should instead fix the misuses of error_setg_win32().
>
>
> This will be more complicated.
> 1. I don't know what code was returned by the Performance Counters API (system or PDH)
> 2. QGA call error_setg_win32_internal as part of error propagation with different error codes,
>  fix the misuses in this case means have a different error propagator for different error codes (back to 1)
> 3. Also, this means that I need to reimplement error_setg_win32_internal for non-system errors with only
> one difference: "unknown Windows error 0x%x" instead of g_win32_error_message, because anyway
> I need the full error propagation part.
>
>>
>> [...]
>>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-21 10:12       ` Kostiantyn Kostiuk
  2025-07-21 11:23         ` Yan Vugenfirer
@ 2025-07-21 11:53         ` Philippe Mathieu-Daudé
  2025-07-25  9:59           ` Kostiantyn Kostiuk
  1 sibling, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-21 11:53 UTC (permalink / raw)
  To: Kostiantyn Kostiuk, Markus Armbruster
  Cc: qemu-devel, Yan Vugenfirer, Daniel Berrangé

On 21/7/25 12:12, Kostiantyn Kostiuk wrote:
> 
> 
> On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <armbru@redhat.com 
> <mailto:armbru@redhat.com>> wrote:
> 
>     Kostiantyn Kostiuk <kkostiuk@redhat.com
>     <mailto:kkostiuk@redhat.com>> writes:
> 
>      > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster
>     <armbru@redhat.com <mailto:armbru@redhat.com>> wrote:
>      >
>      >> Kostiantyn Kostiuk <kkostiuk@redhat.com
>     <mailto:kkostiuk@redhat.com>> writes:
>      >>
>      >> > g_win32_error_message - translate a Win32 error code
>      >> > (as returned by GetLastError()) into the corresponding message.
>      >> >
>      >> > In the same time, we call error_setg_win32_internal with
>      >> > error codes from different Windows componets like VSS or
>      >> > Performance monitor that provides different codes and
>      >> > can't be converted with g_win32_error_message.
>      >>
>      >> Are these error codes from GetLastError()?
>      >>
>      >
>      > No.
>      > VSS functions directly return an error code.
>      > Section: Return value -
>      > https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-
>     vsbackup-ivssbackupcomponents-addtosnapshotset <https://
>     learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-
>     ivssbackupcomponents-addtosnapshotset>
>      >
>      > Performance Counters API can return a system error code or a PDH
>     error code.
>      > Section: Return value -
>      > https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-
>     pdhopenqueryw <https://learn.microsoft.com/en-us/windows/win32/api/
>     pdh/nf-pdh-pdhopenqueryw>
>      > System error code = GetLastError, PDH error code, something else.
>      >
>      > https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-
>     error-codes <https://learn.microsoft.com/en-us/windows/win32/
>     perfctrs/pdh-error-codes>
>      > FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.
> 
>     The error code error_setg_win32() takes is passed to
>     g_win32_error_message().  Contract:
> 
>          g_win32_error_message ()
> 
>          gchar *
>          g_win32_error_message (gint error);
> 
>          Translate a Win32 error code (as returned by GetLastError() or
>          WSAGetLastError()) into the corresponding message.  The message is
>          either language neutral, or in the thread's language, or the user's
>          language, the system's language, or US English (see docs for
>          FormatMessage()).  The returned string is in UTF-8.  It should be
>          deallocated with g_free().
> 
>          Parameters
> 
>              error error code.
> 
>          Returns
> 
>              newly-allocated error message
> 
>     https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-
>     Compatibility-Functions.php#g-win32-error-message <https://
>     www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-
>     Functions.php#g-win32-error-message>
> 
>     Passing error codes from sources other than GetLastError() or
>     WSAGetLastError() violates this contract.
> 
>     Apparently, g_win32_error_message() returns NULL then.  This is not
>     documented behavior.
> 
> 
> It returns an empty string, not NULL.
> https://gitlab.gnome.org/GNOME/glib/-/blob/ 
> a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216 <https:// 
> gitlab.gnome.org/GNOME/glib/-/blob/ 
> a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216>

Worth reporting to GLib IMHO.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-21 11:53         ` Philippe Mathieu-Daudé
@ 2025-07-25  9:59           ` Kostiantyn Kostiuk
  2025-07-28  9:47             ` Philippe Mathieu-Daudé
  2025-07-28 11:12             ` Markus Armbruster
  0 siblings, 2 replies; 12+ messages in thread
From: Kostiantyn Kostiuk @ 2025-07-25  9:59 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Markus Armbruster
  Cc: qemu-devel, Yan Vugenfirer, Daniel Berrangé

[-- Attachment #1: Type: text/plain, Size: 4193 bytes --]

Issue reported to GLib https://gitlab.gnome.org/GNOME/glib/-/issues/3740
PR with fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716

@Markus Armbruster <armbru@redhat.com>
Based on the documentation from this PR, do you have any other objections
to this patch?

Best Regards,
Konstantin Kostiuk.


On Mon, Jul 21, 2025 at 2:53 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 21/7/25 12:12, Kostiantyn Kostiuk wrote:
> >
> >
> > On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <armbru@redhat.com
> > <mailto:armbru@redhat.com>> wrote:
> >
> >     Kostiantyn Kostiuk <kkostiuk@redhat.com
> >     <mailto:kkostiuk@redhat.com>> writes:
> >
> >      > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster
> >     <armbru@redhat.com <mailto:armbru@redhat.com>> wrote:
> >      >
> >      >> Kostiantyn Kostiuk <kkostiuk@redhat.com
> >     <mailto:kkostiuk@redhat.com>> writes:
> >      >>
> >      >> > g_win32_error_message - translate a Win32 error code
> >      >> > (as returned by GetLastError()) into the corresponding message.
> >      >> >
> >      >> > In the same time, we call error_setg_win32_internal with
> >      >> > error codes from different Windows componets like VSS or
> >      >> > Performance monitor that provides different codes and
> >      >> > can't be converted with g_win32_error_message.
> >      >>
> >      >> Are these error codes from GetLastError()?
> >      >>
> >      >
> >      > No.
> >      > VSS functions directly return an error code.
> >      > Section: Return value -
> >      > https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-
> >     vsbackup-ivssbackupcomponents-addtosnapshotset <https://
> >     learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-
> >     ivssbackupcomponents-addtosnapshotset>
> >      >
> >      > Performance Counters API can return a system error code or a PDH
> >     error code.
> >      > Section: Return value -
> >      > https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-
> >     pdhopenqueryw <https://learn.microsoft.com/en-us/windows/win32/api/
> >     pdh/nf-pdh-pdhopenqueryw>
> >      > System error code = GetLastError, PDH error code, something else.
> >      >
> >      > https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-
> >     error-codes <https://learn.microsoft.com/en-us/windows/win32/
> >     perfctrs/pdh-error-codes>
> >      > FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.
> >
> >     The error code error_setg_win32() takes is passed to
> >     g_win32_error_message().  Contract:
> >
> >          g_win32_error_message ()
> >
> >          gchar *
> >          g_win32_error_message (gint error);
> >
> >          Translate a Win32 error code (as returned by GetLastError() or
> >          WSAGetLastError()) into the corresponding message.  The message
> is
> >          either language neutral, or in the thread's language, or the
> user's
> >          language, the system's language, or US English (see docs for
> >          FormatMessage()).  The returned string is in UTF-8.  It should
> be
> >          deallocated with g_free().
> >
> >          Parameters
> >
> >              error error code.
> >
> >          Returns
> >
> >              newly-allocated error message
> >
> >     https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-
> >     Compatibility-Functions.php#g-win32-error-message <https://
> >     www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-
> >     Functions.php#g-win32-error-message>
> >
> >     Passing error codes from sources other than GetLastError() or
> >     WSAGetLastError() violates this contract.
> >
> >     Apparently, g_win32_error_message() returns NULL then.  This is not
> >     documented behavior.
> >
> >
> > It returns an empty string, not NULL.
> > https://gitlab.gnome.org/GNOME/glib/-/blob/
> > a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216 <https://
> > gitlab.gnome.org/GNOME/glib/-/blob/
> > a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216>
>
> Worth reporting to GLib IMHO.
>
>

[-- Attachment #2: Type: text/html, Size: 7322 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-25  9:59           ` Kostiantyn Kostiuk
@ 2025-07-28  9:47             ` Philippe Mathieu-Daudé
  2025-07-28  9:58               ` Kostiantyn Kostiuk
  2025-07-28 11:12             ` Markus Armbruster
  1 sibling, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-28  9:47 UTC (permalink / raw)
  To: Kostiantyn Kostiuk, Markus Armbruster
  Cc: qemu-devel, Yan Vugenfirer, Daniel Berrangé

On 25/7/25 11:59, Kostiantyn Kostiuk wrote:
> Issue reported to GLib https://gitlab.gnome.org/GNOME/glib/-/issues/3740 
> <https://gitlab.gnome.org/GNOME/glib/-/issues/3740>
> PR with fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716 
> <https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716>

Even already merged, thank you!



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-28  9:47             ` Philippe Mathieu-Daudé
@ 2025-07-28  9:58               ` Kostiantyn Kostiuk
  0 siblings, 0 replies; 12+ messages in thread
From: Kostiantyn Kostiuk @ 2025-07-28  9:58 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Markus Armbruster, qemu-devel, Yan Vugenfirer,
	Daniel Berrangé

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

Based on the following part from the new docs
* If a human readable message cannot be found for the given @error, an empty
* string is returned.
Do you have any objections to this patch?

Best Regards,
Konstantin Kostiuk.


On Mon, Jul 28, 2025 at 12:48 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 25/7/25 11:59, Kostiantyn Kostiuk wrote:
> > Issue reported to GLib https://gitlab.gnome.org/GNOME/glib/-/issues/3740
> > <https://gitlab.gnome.org/GNOME/glib/-/issues/3740>
> > PR with fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716
> > <https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716>
>
> Even already merged, thank you!
>
>

[-- Attachment #2: Type: text/html, Size: 2566 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] util: win32: Write hex value when can't get error message
  2025-07-25  9:59           ` Kostiantyn Kostiuk
  2025-07-28  9:47             ` Philippe Mathieu-Daudé
@ 2025-07-28 11:12             ` Markus Armbruster
  1 sibling, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2025-07-28 11:12 UTC (permalink / raw)
  To: Kostiantyn Kostiuk
  Cc: Philippe Mathieu-Daudé, qemu-devel, Yan Vugenfirer,
	Daniel Berrangé

Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:

> Issue reported to GLib https://gitlab.gnome.org/GNOME/glib/-/issues/3740
> PR with fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716
>
> @Markus Armbruster <armbru@redhat.com>
> Based on the documentation from this PR, do you have any other objections
> to this patch?

I can't see that the commit invalidates my objection.

The revised contract still specifies the error code comes from
GetLastError() or WSAGetLastError().  Passing anything else still
violates it.

What can go wrong when we pass some other integer?

Say we pass EINVAL.  It's 22 on Linux.  Interpreted as Windows system
error code (the thing GetLastError() returns), that's ERROR_BAD_COMMAND,
documented as "The device does not recognize the command."[*].
g_win32_error_message() and thus error_setg_win32() will report
confusing nonsense.

Another common integer error code is -1.  This isn't a valid Windows
system error code, so we can expect to hit the "unknown Windows error"
branch.

Perhaps the code is so confused about error codes that passing them to
an appropriate function is impractical, and taking our chances with
g_win32_error_message() is the best we can do.  Wouldn't exactly inspire
confidence in the soundness of QEMU's Windows code.




[*] https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-07-28 11:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-17 14:59 [PATCH] util: win32: Write hex value when can't get error message Kostiantyn Kostiuk
2025-07-17 15:28 ` Yan Vugenfirer
2025-07-19  6:27 ` Markus Armbruster
2025-07-21  7:07   ` Kostiantyn Kostiuk
2025-07-21  9:22     ` Markus Armbruster
2025-07-21 10:12       ` Kostiantyn Kostiuk
2025-07-21 11:23         ` Yan Vugenfirer
2025-07-21 11:53         ` Philippe Mathieu-Daudé
2025-07-25  9:59           ` Kostiantyn Kostiuk
2025-07-28  9:47             ` Philippe Mathieu-Daudé
2025-07-28  9:58               ` Kostiantyn Kostiuk
2025-07-28 11:12             ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).