Re: [gnome-db] gda_holder_set_value_static_str

[Massimo Cora' <maxcvs email it>]

Hi Vivien,

Vivien Malerba wrote:
> * 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.

oh, sorry. It was a typo. It's corrected now. It's right that the
function returns the current holder->priv->value.

> * whenever NULL is retuned and if it's an error, then g_set_error must
> be set.
>  

I set the message "Change refused by signal callback." if the change
signal fails to propagate. Did you mean this?

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

ok to avoid problems in values management now I copy the whole GValue,
even if static, into a fresh new allocated GValue. User should take care
of freeing it. I wrote a comment on the doc of gda_holder_copy (),
because a g_print () there could decrease performances. Let me know if
it's ok like this or I'll change it.


thanks and regards,
Massimo




Index: libgda/gda-holder.c
===================================================================
--- libgda/gda-holder.c	(revision 3224)
+++ libgda/gda-holder.c	(working copy)
@@ -295,6 +295,9 @@
  * @orig: a #GdaHolder object to copy
  *
  * Copy constructor.
+ * 
+ * Note1: if @orig is set with a static value (see #gda_holder_take_static_value ()) 
+ * its copy will have a fresh new allocated GValue, so that user should free it when done.
  *
  * Returns: a new #GdaHolder object
  */
@@ -329,7 +332,7 @@
 		/* direct settings */
 		holder->priv->invalid_forced = orig->priv->invalid_forced;
 		holder->priv->valid = orig->priv->valid;
-		holder->priv->is_freeable = orig->priv->is_freeable;
+		holder->priv->is_freeable = TRUE;
 		holder->priv->default_forced = orig->priv->default_forced;	
 		if (orig->priv->value)
 			holder->priv->value = gda_value_copy (orig->priv->value);
@@ -1001,7 +1004,7 @@
 		newvalid = FALSE;
 		changed = TRUE;
 	}
-/*
+
 #ifdef DEBUG_HOLDER
 	g_print ("Changed holder %p (%s): value %s --> %s \t(type %d -> %d) VALID: %d->%d CHANGED: %d\n", 
 		 holder, holder->priv->id,
@@ -1011,7 +1014,6 @@
 		 value ? G_VALUE_TYPE (value) : 0, 
 		 was_valid, newvalid, changed);
 #endif
-*/
 	
 
 	/* end of procedure if the value has not been changed, after calculating the holder's validity */
@@ -1019,7 +1021,7 @@
 		holder->priv->invalid_forced = FALSE;
 		holder->priv->valid = newvalid;
 #ifdef DEBUG_HOLDER		
-		g_print ("Holder is not changed, returning NULL\n");
+		g_print ("Holder is not changed, returning %p\n", holder->priv->value);
 #endif		
 
 		/* set the changed status */
@@ -1036,6 +1038,9 @@
 	if (lerror) {
 		/* change refused by signal callback */
 		g_propagate_error (error, lerror);
+		
+		g_set_error (error, GDA_HOLDER_ERROR, GDA_HOLDER_VALUE_TYPE_ERROR,
+				     _("Change refused by signal callback."));
 		return NULL;
 	}
 
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 3224)
+++ ChangeLog	(working copy)
@@ -1,3 +1,10 @@
+2008-10-03  Massimo Cora'  <maxcvs email it>
+
+	* libgda/gda-holder.c (gda_holder_copy),
+	(real_gda_holder_set_const_value):
+	fixed typo in debug message. After a gda_holder_copy () call the GValue, even
+	if static, is copied totally so that user should free it when finished to use.
+
 2008-10-03  Vivien Malerba <malerba gnome-db org>
 
 	* providers/postgres/gda-postgres-meta.c: column's GType and DBMS type reporting