Re: Inclusion of baobab in gnome-utils

[Kjartan Maraas <kmaraas broadpark no>]

tir, 11,.04.2006 kl. 11.29 -0600, skrev Elijah Newren: 
> On 4/11/06, Emmanuele Bassi <ebassi gmail com> wrote:
> > Hi all;
> >
> > [I really don't know if this requires a full module proposal or not, but
> > anyway, here we go]
> >
> > I'd like to propose the inclusion of Baobab[1] in the gnome-utils
> > package.
> 
> Given that it's not a new external dependency or a new module of its
> own, you can go ahead and just include it.  However, it is cool and
> helpful to hear your plans (much like how Luis has been pinging people
> for plans when they announce they are branching), so it's basically a
> useful informational announcement.  :-)
> 
After looking at it a bit I've found the following things I'd suggest be
improved first:

- be consistent in coding style. There's a lot of inconsistencies and
wrong indentation etc in there
- eu.po needs converting to UTF-8
- incorporate the current GNOME goals?
- fix all critical warnings
- go through the valgrind report here:
http://www.gnome.org/~kmaraas/baobab-valgrind.log

Here's a few comments from running it through the intel compiler too:

[kmaraas localhost baobab]$ cvs -z3 diff -up src
cvs server: Diffing src
Index: src/baobab-remote-connect-dialog.c
===================================================================
RCS file: /cvs/gnome/baobab/src/baobab-remote-connect-dialog.c,v
retrieving revision 1.2
diff -u -p -r1.2 baobab-remote-connect-dialog.c
--- src/baobab-remote-connect-dialog.c  19 Nov 2005 15:36:14 -0000      1.2
+++ src/baobab-remote-connect-dialog.c  11 Apr 2006 18:15:09 -0000
@@ -125,6 +125,7 @@ remote_connect (BaobabRemoteConnectDialo
                                (_("\"%s\" is not a valid location."),
                                 uri);

+                       /* Use the passed in parameter? */
                        GtkWidget* dialog = gtk_message_dialog_new(NULL,
                                GTK_DIALOG_DESTROY_WITH_PARENT,
                                GTK_MESSAGE_ERROR,
@@ -147,6 +148,7 @@ remote_connect (BaobabRemoteConnectDialo

                server = gtk_editable_get_chars (GTK_EDITABLE (dialog->details->server_entry), 0, -1);
                if (strlen (server) == 0) {
+                       /* Use the passed in parameter? */
                        GtkWidget* dialog = gtk_message_dialog_new(NULL,
                                GTK_DIALOG_DESTROY_WITH_PARENT,
                                GTK_MESSAGE_ERROR,
@@ -297,6 +299,7 @@ remote_connect (BaobabRemoteConnectDialo
                gnome_vfs_uri_unref (vfs_uri);
        }

+       /* icon is never used, is this dead code? */
        if (g_str_has_prefix (uri, "smb:")) {
                icon = "gnome-fs-smb";
        } else if (g_str_has_prefix (uri, "ssh:") ||
Index: src/baobab.c
===================================================================
RCS file: /cvs/gnome/baobab/src/baobab.c,v
retrieving revision 1.30
diff -u -p -r1.30 baobab.c
--- src/baobab.c        11 Apr 2006 15:19:24 -0000      1.30
+++ src/baobab.c        11 Apr 2006 18:15:09 -0000
@@ -41,7 +41,7 @@ static GQueue * iterstack=NULL;
 static void prepare_firstcol(GString*, struct BaobabSearchRet* );
 static gchar* get_owner(uid_t );
 static gchar* get_last_mod(time_t );
-gboolean filter_adv_search(struct BaobabSearchRet *);
+static gboolean filter_adv_search(struct BaobabSearchRet *);
 static void initialize_search_variables(void);
 static void baobab_init(void);
 static void baobab_close(void);
Index: src/bb_util.c
===================================================================
RCS file: /cvs/gnome/baobab/src/bb_util.c,v
retrieving revision 1.25
diff -u -p -r1.25 bb_util.c
--- src/bb_util.c       11 Apr 2006 15:19:24 -0000      1.25
+++ src/bb_util.c       11 Apr 2006 18:15:09 -0000
@@ -366,7 +366,8 @@ baobab_check_dir(const gchar * dirname)
        gboolean bret = TRUE;

        info = gnome_vfs_file_info_new();
-       result=gnome_vfs_get_file_info(dirname, info,GNOME_VFS_FILE_INFO_DEFAULT |
+       /* Should result be checked somewhere? */
+       result = gnome_vfs_get_file_info(dirname, info,GNOME_VFS_FILE_INFO_DEFAULT |
                                GNOME_VFS_FILE_INFO_FOLLOW_LINKS);

        if (info->type != GNOME_VFS_FILE_TYPE_DIRECTORY)
@@ -864,6 +865,7 @@ trash_file(const gchar*  filename)
                response = messageyesno(mess,baobab.window);
                g_free(mess);
                if (response == GTK_RESPONSE_YES) {
+                       /* reuse result declared above? */
                        GnomeVFSResult result;
                        if (!g_file_test (filename, G_FILE_TEST_IS_DIR)) {
                                        result=gnome_vfs_unlink (filename);
@@ -873,6 +875,7 @@ trash_file(const gchar*  filename)
                                }

                        if (result != GNOME_VFS_OK) {
+                       /* reuse the one declared above? */
                        gchar * mess;

                        mess = g_strdup_printf (_("Deleting <b>%s</b> failed: %s."),
Index: src/bbthread.c
===================================================================
RCS file: /cvs/gnome/baobab/src/bbthread.c,v
retrieving revision 1.20
diff -u -p -r1.20 bbthread.c
--- src/bbthread.c      18 Dec 2005 18:48:16 -0000      1.20
+++ src/bbthread.c      11 Apr 2006 18:15:09 -0000
@@ -150,6 +150,7 @@ loopdir(GnomeVFSURI *vfs_uri_dir, guint
                        if (baobab.STOP_SCANNING) goto exit;
                        if ((info->valid_fields & GNOME_VFS_FILE_INFO_FIELDS_TYPE) == 0)
                                        {       continue;
+                                               /* the below is unreachable */
                                                gnome_vfs_file_info_list_free (file_list);
                                                goto exit;
                                                }

Cheers
Kjartan