[Tracker] Checking out some miner code while trying to figure out a crash report..

[Philip Van Hoof <spam pvanhoof be>]

Hey Martyn, Carlos,

In libinotify_monitor_event_cb :

Line ~922:

        other_file = NULL;

        if (!str1) {
                str1 = g_file_get_path (file);
        }

        if (other_file) {
                str2 = g_file_get_path (other_file);
        } else {
                str2 = NULL;
        }



Isn't in this code the variable other_file always NULL?


next Line ~975:

                data = g_hash_table_lookup (monitor->private->event_pairs,
                                            GUINT_TO_POINTER (cookie));

                if (!data) {
                        data = event_data_new (file, event_type);

                        g_hash_table_insert (monitor->private->event_pairs,
                                             GUINT_TO_POINTER (cookie),
                                             data);
                } else {
                        other_file = data->file;
                }

So ok, other_file can be assigned to data->file. But ...

Then at line ~1083:


                } else if (other_file) {
                        g_signal_emit (monitor,
                                       signals[ITEM_MOVED], 0,
                                       file,
                                       other_file,
                                       is_directory, 
                                       TRUE);
                        
                        if (is_directory) {
                                tracker_monitor_update (monitor, file, other_file);
                        }

                        g_hash_table_remove (monitor->private->event_pairs,
                                             GUINT_TO_POINTER (cookie));
                }


So let's look at that tracker_monitor_update:

Line ~1415:


static void
tracker_monitor_update (TrackerMonitor *monitor,
                        GFile          *old_file,
                        GFile          *new_file)
{
        gpointer file_monitor;

        file_monitor = g_hash_table_lookup (monitor->private->monitors, old_file);

        if (!file_monitor) {
                gchar *path;

                path = g_file_get_path (old_file);
                g_warning ("No monitor was found for directory:'%s'", path);
                g_free (path);

                return;
        }

        /* Replace key in monitors hashtable */
        g_hash_table_steal (monitor->private->monitors, old_file);
        g_hash_table_insert (monitor->private->monitors, new_file, file_monitor);
}


So let's look at that monitor->private->monitors's GDestroyNotify-s

Line ~250:

        /* Create monitors table for this module */
        priv->monitors =
                g_hash_table_new_full (g_file_hash,
                                       (GEqualFunc) g_file_equal,
                                       (GDestroyNotify) g_object_unref,
                                       (GDestroyNotify) libinotify_monitor_cancel);


This means that the key (new_file = other_file at the caller) is
unreferenced whenever g_hash_table_insert overwrites the slot in the
hashtable.

Doesn't that mean that it should be like this instead?

g_hash_table_insert (monitor->private->monitors, 
                     g_object_ref (new_file), 
                     file_monitor);

Otherwise will data->file become unreferenced (dangling pointer)
whenever either the hashtable is destroyed or the key is replaced with a
GFile that is g_file_equal.

I'm just wondering because this would explain a bug report by Tshepang.

-- 
Philip Van Hoof, freelance software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://pvanhoof.be/blog
http://codeminded.be