Patch for bug #103174

[Derek Atkins]

Hi,

Nigel Titley <nigel at titley.com> writes:

> Please can someone check and apply this

First, if you could supply unified diffs instead of context diffs
that would be appreciated.  Granted, context diffs are OK, but
I find unified diffs easier to read (and shorter)...

Having said that, I've got a question about the last hunk of
the patch:

> ***************
> *** 240,250 ****
>   void
>   gnc_basic_cell_set_value_internal (BasicCell *cell, const char *value)
>   {
>     if (value == NULL)
>       value = "";
> ! 
>     g_free (cell->value);
> !   cell->value = g_strdup (value);
>   
>     g_free (cell->value_w);
>     cell->value_len = gnc_mbstowcs (&cell->value_w, cell->value);
> --- 244,258 ----
>   void
>   gnc_basic_cell_set_value_internal (BasicCell *cell, const char *value)
>   {
> +   char *lcl_value; /* Used to store a temporary version of value */
> + 
>     if (value == NULL)
>       value = "";
> !   /* Save value, in case it is the same location as cell->value */
> !   lcl_value = g_strdup (value);
>     g_free (cell->value);
> !   /* Finally set cell->value from the local copy */
> !   cell->value = lcl_value;
>   
>     g_free (cell->value_w);
>     cell->value_len = gnc_mbstowcs (&cell->value_w, cell->value);

By my reading of this the only change you're making is that you're
storing the g_strdup(value) into a local variable before you
g_free(cell->value) and then set cell->value to the local var.

I don't understand why this makes a difference.  Functionally the
setting should be the same:

        g_free(foo);
        foo = g_strdup(value);

should be the same as:
        
        temp = g_strdup(value);
        g_free(foo);
        foo = temp;

Am I missing something in your patch?

-derek

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available