On Mon, Jul 21, 2025 at 1:12 PM Kostiantyn Kostiuk <kkost...@redhat.com> wrote: > > > > 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.
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. > >> >> [...] >>
