GNOME Bugzilla – Bug 653185
Use device_automount_hit device property to determine if should_amount is set
Last modified: 2011-07-11 15:42:03 UTC
Two bugs for reference: https://bugs.freedesktop.org/show_bug.cgi?id=38535 https://bugzilla.gnome.org/show_bug.cgi?id=653184 This patch adds the g_gdu_drive_device_automount_policy_always() function and calls it in update_volume() to determine if should_automount is set.
Created attachment 190461 [details] [review] Patch 1/1 - monitor/gdu/ggduvolume.c
Review of attachment 190461 [details] [review]: This patch doesn't account for the case where device_auto_mount_hint is "never". I will attach a more complete patch.
Created attachment 190835 [details] [review] Patch to monitor/gdu/ggduvolume.c that adds support for the device_auto_mount_hint property.
Review of attachment 190835 [details] [review]: references to auto_mount_hint should be changed to automount_hint.
Created attachment 191005 [details] [review] Patch to monitor/gdu/ggduvolume.c that adds support for the device_automount_hint property.
Review of attachment 191005 [details] [review]: ::: monitor/gdu/ggduvolume.c @@ +142,3 @@ + return rc; +} +} It's nicer to write these functions this way { if (g_strcmp0 (gdu_device_get_automount_hint (drive_device), "never") == 0) return TRUE; return FALSE; } @@ +448,3 @@ + volume->should_automount = TRUE; + } + g_gdu_drive_device_is_flash (drive_device) || It's not considered good form to compare a boolean to either TRUE or FALSE - use the idiom "if (!x)" instead of "if (x == FALSE)" or "if (x)" instead of "if (x == TRUE)". I also don't like how the patch changes so many lines - it makes it hard to review.. instead, I think if (g_gdu_drive_device_automount_policy_always (drive_device) || g_strcmp0 (connection_interface, "usb") == 0 || g_strcmp0 (connection_interface, "firewire") == 0 || g_strcmp0 (connection_interface, "sdio") == 0 || g_gdu_drive_device_is_flash (drive_device) || gdu_device_is_optical_disc (drive_device)) { if (!g_gdu_drive_device_automount_policy_never (drive_device)) volume->should_automount = TRUE; } leads to a much easier to review patch
Hmm, I also think it's wrong that we're looking at the drive_device (e.g. /dev/sda) - we should be looking at the device file in question (e.g. /dev/sda2).
OK, I committed this patch http://git.gnome.org/browse/gvfs/commit/?id=493ee806eba214a748d064b43c10882d76ee1492 Please test it and let me know if it works. Thanks.