* [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).