Re: [gnome-db] gda_holder_set_value_static_str

2008/10/2 Massimo Cora' <maxcvs email it>
Hi Vivien,

Vivien Malerba wrote:
> I've applied the patch because it does still pass the checks, but I've
> got a few remarks which, I'm sure you can correct quickly ;)
> * once the is_freeable flag is set to FALSE, there is no way it can be
> back to TRUE

it can be reset to TRUE if a user calls gda_holder_take_value ().

> * the  real_gda_holder_set_const_value() function seems to return NULL
> all the time (the inline doc is not helpfull)

ok I slightly modified the doc and added a gboolean value_changed so
that we always return the previous stored value, or NULL in case of error.

I've just committed your patch!
However, I still have some small issues:
* At line 1022, the value has not changed, and the debug message says that the function returns NULL when in fact it returns holder->priv->value.
* whenever NULL is retuned and if it's an error, then g_set_error must be set.

> * I'm not sure the copy function is correct because it both copies the
> is_freeable flag and the priv->value

well, say for instance that we create a GValue with a static string. We
should copy it with the is_freeable flag set to false, or we may have
some problems trying to free its value.
I see we don't use it into gda-holder.c or within the *take_value
functions. Probably we should warn user of this and avoid to copy the

Yes, printing a warning if some mem leak is about to occur is a good idea.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]