Re: PATCH: anti memleak part 2 [PeterBloomfield@MindSpring.com]

[Peter Bloomfield <PeterBloomfield MindSpring com>]

---

• From: Peter Bloomfield <PeterBloomfield MindSpring com>
• To: Ali Akcaagac <ali akcaagac stud fh-wilhelmshaven de>
• Subject: Re: PATCH: anti memleak part 2
• Date: Thu, 18 Oct 2001 12:41:47 -0400

On 2001.10.18 11:41 Ali Akcaagac wrote:
> jo, subject says everything. please REVIEW the patch before
> applying to CVS..
...
> diff -ruN balsa-cvs/src/save-restore.c balsa/src/save-restore.c
> --- balsa-cvs/src/save-restore.c	Thu Oct 18 16:38:15 2001
> +++ balsa/src/save-restore.c	Thu Oct 18 17:24:53 2001
...
> @@ -1234,12 +1233,13 @@
>  load_mru(GList **mru)
>  {
>      int count, i;
> -    char tmpkey[32];
> +    gchar *tmpkey;
> 
> -    count=d_get_gint("MRUCount", 0);
> -    for(i=0;i<count;i++) {
> -	sprintf(tmpkey, "MRU%d", i+1);
> -	(*mru)=g_list_append((*mru), gnome_config_get_string(tmpkey));
> +    count = d_get_gint("MRUCount", 0);
> +    for (i = 0; i < count; i++) {
> +	tmpkey = g_strdup_printf("MRU%d", i + 1);
> +	(*mru) = g_list_append((*mru), gnome_config_get_string(tmpkey));
> +	g_free (tmpkey);
>      }
>  }

While you're changing this...wouldn't it be cleaner to code this as

static GList *
load_mru(GList *mru)
{
     int i;

     for (i = d_get_gint("MRUCount", 0); i > 0; --i) {
         gchar *tmpkey = g_strdup_printf("MRU%d", i);
         mru = g_list_prepend(mru, gnome_config_get_string(tmpkey));
         g_free(tmpkey);
     }

     return mru;
}

and call it with `balsa_app.folder_mru = 
load_mru(balsa_app.folder_mru);'?

...
> diff -ruN balsa-cvs/src/spell-check.c balsa/src/spell-check.c
> --- balsa-cvs/src/spell-check.c	Thu Oct 18 16:38:15 2001
> +++ balsa/src/spell-check.c	Thu Oct 18 17:15:57 2001
...
> @@ -1089,19 +1084,18 @@
>      gboolean correct;
>      gchar *word = NULL;
> 
> -
>      word = gtk_editable_get_chars(GTK_EDITABLE(spell_check->text),
>  				  spell_check->start_pos,
>  				  spell_check->end_pos);
> 
>      if (word) {
> -
>  	if (balsa_app.debug)
>  	    balsa_information(LIBBALSA_INFORMATION_DEBUG,
>  			      "BalsaSpellCheck: Check %s", word);
> 
>  	correct = pspell_manager_check(spell_check->spell_manager, 
> word);
>      } else {
> +	g_free(word);
>  	return TRUE;
>      }

I don't believe this one's necessary: you get to the new `g_free(word)' 
only on the `else' clause of `if (word)'--so it would always be the 
same as `g_free(NULL)', i.e. a no-op.

---