Re: Keep compilable with --disable-string-variables

[Juergen W. <wie...@fr...>]

On Thursday 13 July 2006 17:14 Ethan A Merritt wrote:
> On Thursday 13 July 2006 12:43 am, Juergen Wieferink wrote:
> > While in it, the snippet:
> >
> >     v = add_udv_by_name("GPVAL_TERM");
> >     if (v) {
> > 	v->udv_undef = FALSE;
> > 	Gstring(&v->udv_value,(char*)term->name); /* this can be pointer */
> >     }
> >
> > looks dangerous to me.  The string a variable points to may be freed
> > if the variable is reset.
>
> I take it that you are worried some other bit of code might call
> gpfree_string(add_udv_by_name("GPVAL_TERM")), thus triggering
> an error when it tries to free the static allocation of term->name?
>
> The GPVAL_* variables cannot be over-written by the user,
> and this one bit of code is the only place that gnuplot itself
> sets this variable. So it is safe.

I don't like code like that nevertheless.  But it's not within my
responsibility.

> > Would an additional function Gstring_copy() be useful, which could be
> > called in places like this?
>
> I don't think that is necessary. You can already do this easily:
>
>   Gstring(&v->udv_value, gp_strdup(term->name));

Good point!

> Then you would have to modify the snippet of code above to deal with the
> opposite problem, that unless the previous name is explicitly freed there
> will be a memory leak.

Well, it could be done like the others:

    fill_gpval_string("GPVAL_TERM", term->name);

Never mind.



I do see another problem in fill_gpval_string():


fill_gpval_string(char *var, char *value)
{
#ifdef GP_STRING_VARS
    struct udvt_entry *v = add_udv_by_name(var);
    if (!v)
	return;
    if (v->udv_undef == FALSE && !strcmp((char*)&v->udv_value, value))
	return;
    v->udv_undef = FALSE; 
    gpfree_string(&v->udv_value);
    Gstring(&v->udv_value, gp_strdup(value));
    /* fprintf(stderr, "now it is: |%s|\n", &v->udv_value.v.string_val); */
#endif
}

When this function is invoked with a new variable, *v is newly
allocated and v->udv_value is not initialized.  It may happen to be
(v->udv_value->type == STRING && v->udv_value->string_val != NULL),
in which (quite improbable) case one would trigger a hardly
reproducible segfault.

I'd suggest something like:

Index: eval.c
===================================================================
--- eval.c	(Revision 279)
+++ eval.c	(Arbeitskopie)
@@ -733,8 +733,10 @@
 	return;
     if (v->udv_undef == FALSE && !strcmp((char*)&v->udv_value, value))
 	return;
-    v->udv_undef = FALSE; 
-    gpfree_string(&v->udv_value);
+    if (v->udv_undef)
+        v->udv_undef = FALSE;
+    else
+        gpfree_string(&v->udv_value);
     Gstring(&v->udv_value, gp_strdup(value));
     /* fprintf(stderr, "now it is: |%s|\n", &v->udv_value.v.string_val); */
 #endif



Juergen