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 704882 - GLocalDirectoryMonitorClass mount_notify field is useless
GLocalDirectoryMonitorClass mount_notify field is useless
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2013-07-25 17:15 UTC by Allison Karlitskaya (desrt)
Modified: 2013-10-03 14:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
directory monitor: use the right 'mount_notify' (2.21 KB, patch)
2013-07-25 17:24 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2013-07-25 17:15:06 UTC
We have this field in GLocalDirectoryMonitorClass:

  gboolean mount_notify;

By default, it is set to FALSE, in g_local_directory_monitor_class_init():

  klass->mount_notify = FALSE;

But some of the backends (like inotify) change it to TRUE:

  local_directory_monitor_class->mount_notify = TRUE;


I *think* it is supposed to mean "this backend will emit mount-related events, so no need for you to emulate them yourself.


Unfortunately, the check for this in g_local_directory_monitor_constructor:


  klass = G_LOCAL_DIRECTORY_MONITOR_CLASS (g_type_class_peek (G_TYPE_LOCAL_DIRECTORY_MONITOR));

...

  if (!klass->mount_notify &&
      (flags & G_FILE_MONITOR_WATCH_MOUNTS))
    {

get things totally wrong by checking the value of the flag on GLocalDirectoryMonitor's vtable rather than the one of the subclass.


The fix for this seems obvious: check the subclass vtable.  I wonder if this would break something, though, considering we've relied on this incorrect behaviour installing unneeded mount monitors for so long...
Comment 1 Allison Karlitskaya (desrt) 2013-07-25 17:24:14 UTC
Created attachment 250131 [details] [review]
directory monitor: use the right 'mount_notify'

During initialisation of a directory monitor with the
G_FILE_MONITOR_WATCH_MOUNTS flag set, GLocalDirectory monitor will add a
UNIX mount watch in case the file notification backend doesn't support
reporting these events for itself.

Unfortunately, it was performing the check incorrectly, resulting in a
monitor always being added.

Fix that, and add the #define for G_LOCAL_DIRECTORY_MONITOR_GET_CLASS()
that was also missing (since the fix depends on it).
Comment 2 Matthias Clasen 2013-07-29 04:09:40 UTC
Indeed, it looks like mount_notify is about whether the implementation emits
G_FILE_MONITOR_EVENT_UNMOUNTED events.
Comment 3 Matthias Clasen 2013-07-29 04:13:24 UTC
Review of attachment 250131 [details] [review]:

Makes sense. Would be great if we had a test for file monitoring that would actually generate UNMOUNTED events.
Comment 4 Allison Karlitskaya (desrt) 2013-10-03 14:35:37 UTC
Attachment 250131 [details] pushed as 1ddfd9d - directory monitor: use the right 'mount_notify'