Fixed bug with define (see bug reference [ 999546 ] 'define' is broken in ngspice-15). free_pnode() now copes properly with parse trees that reuse leaf nodes.

BUGS

@@ -1,21 +1,6 @@
-This file contais a list of known but not yet fixed bugs in ngspice.
+This file contains a list of known but not yet fixed bugs in ngspice.
 ===========================================================================
 
-Rework15fixed:
-
-* define/let commands: 
-  the following code causes a segmentation fault:
-
-   ngspice 1 -> define max(x,y) ( x gt y ) * x + ( x le y ) * y
-   ngspice 2 -> let z = max(2,3)
-   vec_free: Internal Error: not in plot
-   Segmentation fault (core dumped)
-
-  The problem seems caused by the attempt to free an already freed 
-  memory area in free_pnode and free_vec functions.
-
----------------------------------------------------------------------------
-
 * output redirection:
 
    ngspice 1 -> print v(1) > data.txt

ChangeLog

@@ -1,3 +1,8 @@
+2005-05-26  Steven Borley <steven.borley@virgin.net>
+
+  *  Fixed bug with define (see bug reference [ 999546 ] 'define' is broken 
+     in ngspice-15).
+
 2005-05-23  Steven Borley <steven.borley@virgin.net>
 
   *  Fixed a divide-by-zero bug in the MOS level=2 device model

src/frontend/define.c

@@ -1,6 +1,7 @@
 /**********
 Copyright 1990 Regents of the University of California.  All rights reserved.
 Author: 1985 Wayne A. Christopher, U. C. Berkeley CAD Group
+$Id$
 **********/
 
 /*
@@ -339,25 +340,32 @@ trcopy(struct pnode *tree, char *args, struct pnode *nn)
     } else if (tree->pn_func) {
 
         pn = alloc(struct pnode);
+	pn->pn_use = 0;
+	pn->pn_name = NULL;
         pn->pn_value = NULL;
         /* pn_func are pointers to a global constant struct */	
         pn->pn_func = tree->pn_func;
         pn->pn_op = NULL;
         pn->pn_left = trcopy(tree->pn_left, args, nn);
+	pn->pn_left->pn_use++;
         pn->pn_right = NULL;
         pn->pn_next = NULL;
 
     } else if (tree->pn_op) {
 
         pn = alloc(struct pnode);
+	pn->pn_use = 0;
+	pn->pn_name = NULL;
         pn->pn_value = NULL;
         pn->pn_func = NULL;
         /* pn_op are pointers to a global constant struct */	
         pn->pn_op = tree->pn_op;
         pn->pn_left = trcopy(tree->pn_left, args, nn);
-        if (pn->pn_op->op_arity == 2)
+	pn->pn_left->pn_use++;
+        if (pn->pn_op->op_arity == 2) {
             pn->pn_right = trcopy(tree->pn_right, args, nn);
-	else
+	    pn->pn_right->pn_use++;
+	} else
             pn->pn_right = NULL;
         pn->pn_next = NULL;
     } else {

src/frontend/parse.c

@@ -66,6 +66,7 @@ ft_getpnames(wordlist *wl, bool check)
 
         if (pn) {
             lpn->pn_next = p;
+	    pn->pn_next->pn_use++;
             lpn = p;
         } else
             pn = lpn = p;
@@ -713,7 +714,7 @@ struct func func_uminus = { "minus", cx_uminus };
 
 struct func func_not = { "not", cx_not };
 
-/* Binop node. */
+/* Binary operator node. */
 
 static struct pnode *
 mkbnode(int opnum, struct pnode *arg1, struct pnode *arg2)
@@ -728,16 +729,20 @@ mkbnode(int opnum, struct pnode *arg1, struct pnode *arg2)
         fprintf(cp_err, "mkbnode: Internal Error: no such op num %d\n",
                     opnum);
     p = alloc(struct pnode);
+    p->pn_use = 0;
     p->pn_value = NULL;
+    p->pn_name = NULL;	/* sjb */
     p->pn_func = NULL;
     p->pn_op = o;
     p->pn_left = arg1;
+    if(p->pn_left) p->pn_left->pn_use++;
     p->pn_right = arg2;
+    if(p->pn_right) p->pn_right->pn_use++;
     p->pn_next = NULL;
     return (p);
 }
 
-/* Unop node. */
+/* Unary operator node. */
 
 static struct pnode *
 mkunode(int op, struct pnode *arg)
@@ -754,9 +759,12 @@ mkunode(int op, struct pnode *arg)
                 op);
 
     p->pn_op = o;
+    p->pn_use = 0;
     p->pn_value = NULL;
+    p->pn_name = NULL;	/* sjb */
     p->pn_func = NULL;
     p->pn_left = arg;
+    if(p->pn_left) p->pn_left->pn_use++;
     p->pn_right = NULL;
     p->pn_next = NULL;
     return (p);
@@ -818,11 +826,13 @@ mkfnode(char *func, struct pnode *arg)
     }
 
     p = alloc(struct pnode);
+    p->pn_use = 0;
     p->pn_name = NULL;
     p->pn_value = NULL;
     p->pn_func = f;
     p->pn_op = NULL;
     p->pn_left = arg;
+    if(p->pn_left) p->pn_left->pn_use++;
     p->pn_right = NULL;
     p->pn_next = NULL;
     return (p);
@@ -840,6 +850,7 @@ mknnode(double number)
     p = alloc(struct pnode);
     v = alloc(struct dvec);
     ZERO(v, struct dvec);
+    p->pn_use = 0;
     p->pn_name = NULL;
     p->pn_value = v;
     p->pn_func = NULL;
@@ -875,6 +886,7 @@ mksnode(char *string)
     struct pnode *p;
 
     p = alloc(struct pnode);
+    p->pn_use = 0;
     p->pn_name = NULL;
     p->pn_func = NULL;
     p->pn_op = NULL;
@@ -910,18 +922,32 @@ mksnode(char *string)
     return (p);
 }
 
-
+/* Don't call this directly, always use the free_pnode() macro. 
+   The linked pnodes do not necessarily form a perfect tree as some nodes get
+   reused.  Hence, in this recursive walk trough the 'tree' we only free node
+   that have their pn_use value at zero. Nodes that have pn_use values above
+   zero have the link severed and their pn_use value decremented.
+   In addition, we don't walk past nodes with pn_use values avoid zero, just
+   in case we have a circular reference (this probable does not happen in
+  practice, but it does no harm playing safe) */
 void
-free_pnode(struct pnode *t)
+free_pnode_x(struct pnode *t)
 {
     if (!t)
 	return;
-    free_pnode(t->pn_left);
-    free_pnode(t->pn_right);
-    free_pnode(t->pn_next);
-    tfree(t->pn_name); /* va: it is a copy() of original string, can be free'd */
-    if (t->pn_value)
-        vec_free(t->pn_value); /* patch by Stefan Jones */
-    tfree(t);
+    
+    /* don't walk past node used elsewhere. We decrement the pn_use value here,
+       but the link gets severed by the action of the free_pnode() macro */
+    if(t->pn_use)
+	t->pn_use--;
+    else {
+	free_pnode(t->pn_left);
+	free_pnode(t->pn_right);
+	free_pnode(t->pn_next);
+	tfree(t->pn_name); /* va: it is a copy() of original string, can be free'd */
+	if (t->pn_value)
+	    vec_free(t->pn_value); /* patch by Stefan Jones */
+	tfree(t);
+    }
 }
 

src/frontend/parse.h

@@ -1,6 +1,7 @@
 /*************
  * Header file for parse.c
  * 1999 E. Rouat
+ * $Id$
  ************/
 
 #ifndef _PARSE_H
@@ -10,7 +11,8 @@
 #include <wordlist.h>
 
 struct pnode * ft_getpnames(wordlist *wl, bool check);
-void free_pnode(struct pnode *t);
+#define free_pnode(ptr)  free_pnode_x(ptr); ptr=NULL;
+void free_pnode_x(struct pnode *t);
 
 
 #endif

src/frontend/vectors.c

@@ -1,6 +1,7 @@
 /**********
 Copyright 1990 Regents of the University of California.  All rights reserved.
 Author: 1985 Wayne A. Christopher, U. C. Berkeley CAD Group 
+$Id$
 **********/
 
 /*
@@ -590,7 +591,7 @@ vec_gc(void)
  */
 
 void
-vec_free(struct dvec *v)
+vec_free_x(struct dvec *v)
 {
     struct plot *pl;
     struct dvec *lv;

src/frontend/vectors.h

@@ -1,6 +1,7 @@
 /*************
  * Header file for vectors.c
  * 1999 E. Rouat
+ * $Id$
  ************/
 
 #ifndef VECTORS_H_INCLUDED
@@ -16,7 +17,8 @@ struct dvec * vec_copy(struct dvec *v);
 struct plot * plot_alloc(char *name);
 void vec_new(struct dvec *d);
 void vec_gc(void);
-void vec_free(struct dvec *v);
+#define vec_free(ptr)  vec_free_x(ptr); ptr=NULL
+void vec_free_x(struct dvec *v);
 bool vec_eq(struct dvec *v1, struct dvec *v2);
 char * vec_basename(struct dvec *v);
 void plot_setcur(char *name);

src/include/pnode.h

@@ -1,3 +1,4 @@
+/* $Id$ */
 #ifndef _PNODE_H
 #define _PNODE_H
 
@@ -9,6 +10,7 @@ struct pnode {
     struct pnode *pn_left;	/* Left branch or function argument. */
     struct pnode *pn_right;	/* Right branch. */
     struct pnode *pn_next;	/* For expression lists. */
+    int pn_use;			/* usage counter */
 } ;
 
 #endif