Re: Patch with lots of cleanups for warnings from sparse and the Intel compiler

[Rodrigo Moya <rodrigo novell com>]

On Tue, 2005-11-01 at 22:52 +0100, Kjartan Maraas wrote:
> This does the following:
> 
> - mark code static when possible
> - remove unused code
> - make some parts ANSI compliant
> - fix some format specifiers
> 
some comments:

> Index: capplets/keybindings/gnome-keybinding-properties.c
> @@ -442,7 +442,6 @@ should_show_key (const KeyListEntry *ent
>        return TRUE;
>      else
>        return FALSE;
> -    break;
>    }
this looks ok, although it might make older gcc's to show warnings about
it, AFAIK.

> @@ -191,7 +191,6 @@ transfer_done_tarbz2_idle_cb (gpointer d
>  static void
>  transfer_done_cb (GtkWidget *dlg, gchar *path)
>  {
> -       GtkWidget *dialog;
>         int len = strlen (path);
>         gchar *command,**dir, *first_line, *filename;
>         int status,theme_type;
> @@ -220,6 +219,8 @@ transfer_done_cb (GtkWidget *dlg, gchar 
>                 theme_props->filetype=TARBZ;
>                 g_free (filename);
>         } else {
> +               GtkWidget *dialog;
> +
>                 dialog = gtk_message_dialog_new (NULL,
>                                                 GTK_DIALOG_MODAL,
>                                                 GTK_MESSAGE_ERROR,
> @@ -600,15 +601,15 @@ gnome_theme_installer_run (GtkWidget *pa
>         running_theme_install = TRUE;
>  
>         if (!g_file_test ("/bin/tar", G_FILE_TEST_EXISTS)) {
> -               GtkWidget *dialog;
> +               GtkWidget *error_dialog;
>  
> -               dialog = gtk_message_dialog_new (NULL,
> +               error_dialog = gtk_message_dialog_new (NULL,
>                                                 GTK_DIALOG_MODAL,
>                                                 GTK_MESSAGE_ERROR,
>                                                 GTK_BUTTONS_OK,
>                                                 _("Cannot install
> theme.\nThe tar program is not installed on your system."));
> -               gtk_dialog_run (GTK_DIALOG (dialog));
> -               gtk_widget_destroy (dialog);
> +               gtk_dialog_run (GTK_DIALOG (error_dialog));
> +               gtk_widget_destroy (error_dialog);
>                 return;
>         }
here, why don't you declare one GtkDialog just once, as you were doing
in your previous patch?

> @@ -115,6 +115,5 @@ AcmeVolume *acme_volume_new (void)
>         vol = ACME_VOLUME  (g_object_new (acme_volume_oss_get_type (),
> NULL));
>         return vol;
>  #endif
> -       return ACME_VOLUME  (g_object_new (acme_volume_dummy_get_type
> (), NULL));
>  }
AFAICS, there are code patchs that can reach this last line, so why not
just return NULL there instead of removing the return statement?

The rest looks ok, provided it compiles, so please answer back on these
comments, and once we're set, I think this can be committed to both HEAD
and the gnome-2-12 branch
-- 
Rodrigo Moya <rodrigo novell com>