On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <arm...@redhat.com> wrote:
> Kostiantyn Kostiuk <kkost...@redhat.com> writes: > > > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <arm...@redhat.com> > wrote: > > > >> Kostiantyn Kostiuk <kkost...@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. > [...] > >