After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 777308 - GModule win32: disable error dialog popup
GModule win32: disable error dialog popup
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gmodule
unspecified
Other Windows
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-01-16 08:38 UTC by Tom Schoonjans
Modified: 2017-11-02 15:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GModule win32: disable error dialogs (1.04 KB, patch)
2017-01-16 08:39 UTC, Tom Schoonjans
none Details | Review
GModule win32 suppress error dialogs patch (1.24 KB, patch)
2017-11-02 13:38 UTC, Tom Schoonjans
none Details | Review
GModule win32 suppress error dialogs patch (1.33 KB, patch)
2017-11-02 15:08 UTC, Tom Schoonjans
accepted-commit_now Details | Review

Description Tom Schoonjans 2017-01-16 08:38:44 UTC
When loading a module on win32, a blocking error dialog pops up whenever
the module could not be loaded. This is particularly annoying when
module loading failure is a harmless and expected event...

The attached patch temporarily disables these error dialogs from popping up.
Comment 1 Tom Schoonjans 2017-01-16 08:39:47 UTC
Created attachment 343533 [details] [review]
GModule win32: disable error dialogs
Comment 2 Philip Withnall 2017-11-01 12:59:37 UTC
Review of attachment 343533 [details] [review]:

::: gmodule/gmodule-win32.c
@@ +80,3 @@
 
+  /* suppress error dialog */
+  SetErrorMode(SEM_NOOPENFILEERRORBOX | SEM_FAILCRITICALERRORS);

GLib now requires Windows 7 (https://wiki.gnome.org/Projects/GLib/SupportedPlatforms), so we can (and should) use SetThreadErrorMode() to prevent threading problems with this code fighting with the error mode in other threads.

https://msdn.microsoft.com/en-us/library/windows/desktop/dd553630(v=vs.85).aspx

Also, code-style-wise, please put a space before the first `(` in a function call.

@@ +83,2 @@
   handle = LoadLibraryW (wfilename);
+  SetErrorMode(0);

This should restore the previous set of flags, as returned by the first SetThreadErrorMode() call.
Comment 3 Tom Schoonjans 2017-11-01 13:18:00 UTC
Ok thanks, will make the necessary changes soon.
Comment 4 Tom Schoonjans 2017-11-02 13:38:00 UTC
Created attachment 362825 [details] [review]
GModule win32 suppress error dialogs patch

New patch, with fixes per your previous comments.
Comment 5 Philip Withnall 2017-11-02 14:49:27 UTC
Review of attachment 362825 [details] [review]:

::: gmodule/gmodule-win32.c
@@ +81,3 @@
 
+  /* suppress error dialog */
+  SetThreadErrorMode (SEM_NOOPENFILEERRORBOX | SEM_FAILCRITICALERRORS, &old_mode);

old_mode is only going to be valid if SetThreadErrorMode() succeeds, so you need to check the return value and skip the SetThreadErrorMode(old_mode) call if the first call fails.
Comment 6 Tom Schoonjans 2017-11-02 15:08:33 UTC
Created attachment 362837 [details] [review]
GModule win32 suppress error dialogs patch

Updated with check for SetThreadErrorMode return value
Comment 7 Philip Withnall 2017-11-02 15:34:01 UTC
Review of attachment 362837 [details] [review]:

Looks good to me, thanks.

There’s only slight value in calling set_error(""), given that the caller of g_module_open() can’t detect the error from the SetThreadErrorMode() call. The g_module_close() implementation in gmodule-win32.c does this too, though, so there’s precedent.
Comment 8 Tom Schoonjans 2017-11-02 15:40:51 UTC
Ok thanks for merging it in
Comment 9 Philip Withnall 2017-11-02 15:50:43 UTC
commit 719edde63b1f4756c61c325e8457c7d033be8194 (HEAD -> master, origin/master, origin/HEAD)
Author: Tom Schoonjans <Tom.Schoonjans@diamond.ac.uk>
Date:   Mon Jan 16 11:39:49 2017 +0530

    GModule win32: disable error dialog popup
    
    When loading a module on win32, a blocking error dialog pops up whenever
    the module could not be loaded. This is particularly annoying when
    module loading failure is a harmless and expected event...
    
    This patch temporarily disables these error dialogs from popping up.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777308