Re: [Tracker] [Patch] DBus connection lost handling

[Jamie McCracken <jamiemcc blueyonder co uk>]

Laurent Aguerreche wrote:
> Le lundi 04 septembre 2006 Ã 13:58 +0100, Jamie McCracken a Ãcrit :
> > Jamie McCracken wrote:
> >
> > > (move do_cleanup to tracker-utils.c/.h and make it non static. We can 
> > > probably use tracker->is_running instead of static shutdown variable in 
> > > trackerd.c too?)
> > Actually we will need a tracker->shutdown variable instead of the static 
> > shutdown variable. Shutdown is used to tell threads to stop (when 
> > asleep) while tracker->is_running encourages them to sleep so they are 
> > different.
>
> If I move do_cleanup() in tracker-utils.c, I also need to make
> main_connection a field a tracker structure.
>
>
> I've just added that in trackerd.c:
>
>
> static void
> unregistered_func (DBusConnection *connection,
>                    gpointer        data)
> {
> }
>
>
> static DBusHandlerResult
> local_dbus_connection_monitoring_message_func (DBusConnection
> *connection,
>                                                DBusMessage    *message,
>                                                gpointer        data)
> {
>         /* DBus connection has been lost! */
>         if (dbus_message_is_signal (message, DBUS_INTERFACE_LOCAL,
> "Disconnected")) {
>                 dbus_message_ref (message);
>
>                 tracker_log ("DBus connection has been lost, trackerd will shutdown");
>
>                 tracker->is_running = FALSE;
>                 tracker_end_watching ();
>                 do_cleanup ("DBus connection lost");
>         }
>
>         return DBUS_HANDLER_RESULT_HANDLED;
> }
>
>
> static gboolean
> add_local_dbus_connection_monitoring (DBusConnection *connection)
> {
>         DBusObjectPathVTable dbus_daemon_vtable = {
>                 unregistered_func,
>                 local_dbus_connection_monitoring_message_func,
>                 NULL,
>                 NULL,
>                 NULL,
>                 NULL
>         };
>
>         if (!dbus_connection_register_object_path (connection, DBUS_PATH_LOCAL,
> &dbus_daemon_vtable, NULL)) {
>                 tracker_log ("could not register local D-BUS connection handler");
>                 return FALSE;
>
>         } else {
>                 return TRUE;
>         }
> }
>
>
>
> and I added a call to add_local_dbus_connection_monitoring() in main().
>
> That's all...
>
>
>
> But my patch contains more than that. I have begun to clean code. I
> treated those files:
> * trackerd.{h,c}
> * tracker-db.{h,c}
> * tracker-db-mysql.{h,c}
> * tracker-metadata.{h,c}
> * tracker-dbus.{h,c}
> * tracker-dbus-keywords.{h,c}
> * tracker-dbus-search.{h,c}
> * tracker-utils.{h,c}
>
>
> This patch:
>
> - reduces scope of many local variables;
> - removes some foo = NULL at declaration when there is something like:
>    foo = a_function();
>   after...
> - removes initialization of variables at their declaration (more old-C
> like, so 'old' compilers should easily compile the code :-) )
> - rewrites some loops:
>
>    tmp = foo;
>    while (tmp) {
>       /* many lines ... */
>       tmp = tmp->next;
>    }
>
>   becomes
>
>    for (tmp = foo; tmp; tmp = tmp->next) {
>       /* many lines ... */
>    }
>
> - tries to replace more POSIX functions by corresponding Glib functions:
>   * unlink() => g_unlink()
>   * fopen() => g_open()
>   * opendir() => g_dir_open()
>   * readdir() => g_dir_read_name()
>
>   Unlike readdir(), g_dir_read_name() ignores '.' and '..'. So I added a
> call to tracker_ignore_file() in tracker_get_files() otherwise file
> names that begin with a dot were treated...
>  - replaces '/tmp' by g_get_tmp_dir();
>  - replaces g_strconcat(path,"/",file,NULL) by
> g_build_filename(path,file,NULL);
>  - replaces '/' by G_DIR_SEPARATOR and "/" by G_DIR_SEPARATOR_S
>
> So with this replacements and the use of Glib functions rather than
> directly POSIX ones, trackerd becomes less Linux/Unix centric.
>
> - adds/removes some #include
> - removes many empty lines or spaces, and also added other blank
> lines...
>
>
> This patch is rather big but I think it is 'easy' to read since it only
> does trivial things.
> I would like to see it applied to continue on other files...

yes great - thanks a lot!

At first glance it looks very good but will get back to you shortly once 
I've looked at it more closely (always the case with big patches).



--
Mr Jamie McCracken
http://jamiemcc.livejournal.com/