From d6d3ab502c09c6512b79337b687c42b62e2c1b88 Mon Sep 17 00:00:00 2001
From: Stefan Frederik <stefan.schippers@gmail.com>
Date: Tue, 23 Nov 2021 01:14:49 +0100
Subject: [PATCH] some comments in the code, avoid potential memory leak in
 netlisting code

---
 src/actions.c         | 107 +++++++++++++++++++-----------------------
 src/hilight.c         |   4 +-
 src/netlist.c         |   2 +-
 src/spice_netlist.c   |   2 +-
 src/tedax_netlist.c   |   4 +-
 src/track_memory.awk  |   2 +-
 src/verilog_netlist.c |   4 +-
 src/vhdl_netlist.c    |   9 ++--
 8 files changed, 62 insertions(+), 72 deletions(-)

diff --git a/src/actions.c b/src/actions.c
index 354e2fc3..73e78c9c 100644
--- a/src/actions.c
+++ b/src/actions.c
@@ -451,73 +451,61 @@ void remove_symbol(int j)
   my_free(668, &xctx->sym[j].type);
   my_free(684, &xctx->sym[j].name);
   /*  /20150409 */
-  for(c=0;c<cadlayers;c++)
-  {
-   for(i=0;i<xctx->sym[j].polygons[c];i++)
-   {
-     if(xctx->sym[j].poly[c][i].prop_ptr != NULL) {
-       my_free(669, &xctx->sym[j].poly[c][i].prop_ptr);
-     }
-     my_free(670, &xctx->sym[j].poly[c][i].x);
-     my_free(671, &xctx->sym[j].poly[c][i].y);
-     my_free(672, &xctx->sym[j].poly[c][i].selected_point);
-   }
-   my_free(673, &xctx->sym[j].poly[c]);
-   xctx->sym[j].polygons[c] = 0;
-
-   for(i=0;i<xctx->sym[j].lines[c];i++)
-   {
-    if(xctx->sym[j].line[c][i].prop_ptr != NULL)
-    {
-     my_free(674, &xctx->sym[j].line[c][i].prop_ptr);
+  for(c=0;c<cadlayers;c++) {
+    for(i=0;i<xctx->sym[j].polygons[c];i++) {
+      if(xctx->sym[j].poly[c][i].prop_ptr != NULL) {
+        my_free(669, &xctx->sym[j].poly[c][i].prop_ptr);
+      }
+      my_free(670, &xctx->sym[j].poly[c][i].x);
+      my_free(671, &xctx->sym[j].poly[c][i].y);
+      my_free(672, &xctx->sym[j].poly[c][i].selected_point);
     }
-   }
-   my_free(675, &xctx->sym[j].line[c]);
-   xctx->sym[j].lines[c] = 0;
-
-   for(i=0;i<xctx->sym[j].arcs[c];i++)
-   {
-    if(xctx->sym[j].arc[c][i].prop_ptr != NULL)
-    {
-     my_free(676, &xctx->sym[j].arc[c][i].prop_ptr);
+    my_free(673, &xctx->sym[j].poly[c]);
+    xctx->sym[j].polygons[c] = 0;
+ 
+    for(i=0;i<xctx->sym[j].lines[c];i++) {
+      if(xctx->sym[j].line[c][i].prop_ptr != NULL) {
+        my_free(674, &xctx->sym[j].line[c][i].prop_ptr);
+      }
     }
-   }
-   my_free(677, &xctx->sym[j].arc[c]);
-   xctx->sym[j].arcs[c] = 0;
-
-   for(i=0;i<xctx->sym[j].rects[c];i++)
-   {
-    if(xctx->sym[j].rect[c][i].prop_ptr != NULL)
-    {
-     my_free(678, &xctx->sym[j].rect[c][i].prop_ptr);
+    my_free(675, &xctx->sym[j].line[c]);
+    xctx->sym[j].lines[c] = 0;
+ 
+    for(i=0;i<xctx->sym[j].arcs[c];i++) {
+      if(xctx->sym[j].arc[c][i].prop_ptr != NULL) {
+        my_free(676, &xctx->sym[j].arc[c][i].prop_ptr);
+      }
     }
-   }
-   my_free(679, &xctx->sym[j].rect[c]);
-   xctx->sym[j].rects[c] = 0;
+    my_free(677, &xctx->sym[j].arc[c]);
+    xctx->sym[j].arcs[c] = 0;
+ 
+    for(i=0;i<xctx->sym[j].rects[c];i++) {
+      if(xctx->sym[j].rect[c][i].prop_ptr != NULL) {
+        my_free(678, &xctx->sym[j].rect[c][i].prop_ptr);
+      }
+    }
+    my_free(679, &xctx->sym[j].rect[c]);
+    xctx->sym[j].rects[c] = 0;
   }
-  for(i=0;i<xctx->sym[j].texts;i++)
-  {
-   if(xctx->sym[j].text[i].prop_ptr != NULL)
-   {
-    my_free(680, &xctx->sym[j].text[i].prop_ptr);
-   }
-   if(xctx->sym[j].text[i].txt_ptr != NULL)
-   {
-    my_free(681, &xctx->sym[j].text[i].txt_ptr);
-   }
-   if(xctx->sym[j].text[i].font != NULL)
-   {
-    my_free(682, &xctx->sym[j].text[i].font);
-   }
+  for(i=0;i<xctx->sym[j].texts;i++) {
+    if(xctx->sym[j].text[i].prop_ptr != NULL) {
+      my_free(680, &xctx->sym[j].text[i].prop_ptr);
+    }
+    if(xctx->sym[j].text[i].txt_ptr != NULL) {
+      my_free(681, &xctx->sym[j].text[i].txt_ptr);
+    }
+    if(xctx->sym[j].text[i].font != NULL) {
+      my_free(682, &xctx->sym[j].text[i].font);
+    }
   }
   my_free(683, &xctx->sym[j].text);
   xctx->sym[j].texts = 0;
 
-  save = xctx->sym[j];
+  save = xctx->sym[j]; /* save cleared symbol slot */
   for(i = j + 1; i < xctx->symbols; i++) {
     xctx->sym[i-1] = xctx->sym[i];
   }
-  xctx->sym[xctx->symbols-1] = save;
+  xctx->sym[xctx->symbols-1] = save; /* fill end with cleared symbol slot */
   xctx->symbols--;
 }
 
@@ -525,8 +513,8 @@ void remove_symbols(void)
 {
   int j;
 
-  for(j = 0; j < xctx->instances; j++) { /* must be deleted before symbols are deleted */
-    delete_inst_node(j);
+  for(j = 0; j < xctx->instances; j++) {
+    delete_inst_node(j); /* must be deleted before symbols are deleted */
     xctx->inst[j].ptr = -1; /* clear symbol reference on instanecs */
   }
   for(j=xctx->symbols-1;j>=0;j--) {
@@ -1692,7 +1680,8 @@ void new_wire(int what, double mx_snap, double my_snap)
       xctx->prep_hi_structs = 0;
       if(tclgetboolvar("autotrim_wires")) trim_wires();
       if(s_pnetname || xctx->hilight_nets) {
-        prepare_netlist_structs(0);
+        prepare_netlist_structs(0); /* since xctx->prep_hi_structs==0, do a delete_netlist_structs() first,
+                                     * this clears both xctx->prep_hi_structs and xctx->prep_net_structs. */
         if(!big) {
           bbox(START , 0.0 , 0.0 , 0.0 , 0.0);
           if(s_pnetname || xctx->hilight_nets) {
diff --git a/src/hilight.c b/src/hilight.c
index 02d00d29..fac418fe 100644
--- a/src/hilight.c
+++ b/src/hilight.c
@@ -1315,7 +1315,7 @@ void create_simdata(void)
     xSymbol *symbol = xctx->inst[i].ptr + xctx->sym;
     int npin = symbol->rects[PINLAYER];
     xctx->simdata[i].pin = NULL;
-    my_realloc(61, &xctx->simdata[i].pin, npin * sizeof(struct simdata_pin));
+    if(npin) my_realloc(61, &xctx->simdata[i].pin, npin * sizeof(struct simdata_pin));
     xctx->simdata[i].npin = npin;
     for(j = 0; j < npin; j++) {
       char function[20];
@@ -1342,7 +1342,7 @@ void free_simdata(void)
         my_free(1219, &xctx->simdata[i].pin[j].function);
         my_free(1220, &xctx->simdata[i].pin[j].go_to);
       }
-      my_free(1221, &xctx->simdata[i].pin);
+      if(npin) my_free(1221, &xctx->simdata[i].pin);
     }
     my_free(1222, &xctx->simdata);
   }
diff --git a/src/netlist.c b/src/netlist.c
index 2d54bb95..cdebe74b 100644
--- a/src/netlist.c
+++ b/src/netlist.c
@@ -1302,7 +1302,7 @@ int sym_vs_sch_pins()
 void delete_inst_node(int i)
 {
    int j, rects;
-   if(!xctx->inst[i].node) return;
+   if( xctx->inst[i].ptr == -1 || !xctx->inst[i].node) return;
    rects = (xctx->inst[i].ptr+ xctx->sym)->rects[PINLAYER] +
            (xctx->inst[i].ptr+ xctx->sym)->rects[GENERICLAYER];
    if( rects > 0 )
diff --git a/src/spice_netlist.c b/src/spice_netlist.c
index ac9edc46..dc1d4ec1 100644
--- a/src/spice_netlist.c
+++ b/src/spice_netlist.c
@@ -76,8 +76,8 @@ void hier_psprint(void)  /* netlister driver */
         dbg(1,"--> %s\n", skip_dir( xctx->sch[xctx->currsch]) );
       }
     }
-    my_free(1231, &abs_path);
   }
+  my_free(1231, &abs_path);
   free_hash(subckt_table);
   my_free(1229, &subckt_name);
   my_strncpy(xctx->sch[xctx->currsch] , "", S(xctx->sch[xctx->currsch]));
diff --git a/src/tedax_netlist.c b/src/tedax_netlist.c
index 91e70b9e..5a97a966 100644
--- a/src/tedax_netlist.c
+++ b/src/tedax_netlist.c
@@ -100,13 +100,13 @@ void global_tedax_netlist(int global)  /* netlister driver */
    {
     if( strcmp(get_tok_value(xctx->sym[i].prop_ptr,"tedax_ignore",0),"true")==0 ) continue;
     if(!xctx->sym[i].type) continue;
-    my_strdup(1236, &abs_path, abs_sym_path(xctx->sym[i].name, ""));
+    my_strdup2(1236, &abs_path, abs_sym_path(xctx->sym[i].name, ""));
     if(strcmp(xctx->sym[i].type,"subcircuit")==0 && check_lib(1, abs_path))
     {
       tedax_block_netlist(fd, i);
     }
-    my_free(1237, &abs_path);
    }
+   my_free(1237, &abs_path);
    /*clear_drawing(); */
    my_strncpy(xctx->sch[xctx->currsch] , "", S(xctx->sch[xctx->currsch]));
    xctx->currsch--;
diff --git a/src/track_memory.awk b/src/track_memory.awk
index 30505e24..5eca8f0b 100755
--- a/src/track_memory.awk
+++ b/src/track_memory.awk
@@ -75,7 +75,7 @@ END{
     leak+= address[i]
     print "  address[ " i ", " idx[i] " ]= " address[i]
     if(show_source) {
-      pipe = "egrep -n 'my_(malloc|calloc|realloc|free)\(" idx[i] ",' *.c xschem.h"
+      pipe = "egrep -n 'my_(malloc|calloc|realloc|free|strdup|strdup2)\(" idx[i] ",' *.c xschem.h"
       while( pipe | getline a) print "    " a
       close(pipe)
     }
diff --git a/src/verilog_netlist.c b/src/verilog_netlist.c
index f0cac99a..c335c22a 100644
--- a/src/verilog_netlist.c
+++ b/src/verilog_netlist.c
@@ -296,7 +296,7 @@ void global_verilog_netlist(int global)  /* netlister driver */
    {
     if( strcmp(get_tok_value(xctx->sym[i].prop_ptr,"verilog_ignore",0),"true")==0 ) continue;
     if(!xctx->sym[i].type) continue;
-    my_strdup(1234, &abs_path, abs_sym_path(xctx->sym[i].name, ""));
+    my_strdup2(1234, &abs_path, abs_sym_path(xctx->sym[i].name, ""));
     if(strcmp(xctx->sym[i].type,"subcircuit")==0 && check_lib(1, abs_path)) {
       /* xctx->sym can be SCH or SYM, use hash to avoid writing duplicate subckt */
       my_strdup(328, &subckt_name, get_cell(xctx->sym[i].name, 0));
@@ -312,8 +312,8 @@ void global_verilog_netlist(int global)  /* netlister driver */
             verilog_block_netlist(fd, i);
       }
     }
-    my_free(1235, &abs_path);
    }
+   my_free(1235, &abs_path);
    free_hash(subckt_table);
    my_free(1073, &subckt_name);
    my_strncpy(xctx->sch[xctx->currsch] , "", S(xctx->sch[xctx->currsch]));
diff --git a/src/vhdl_netlist.c b/src/vhdl_netlist.c
index 7fb96881..aa534df0 100644
--- a/src/vhdl_netlist.c
+++ b/src/vhdl_netlist.c
@@ -535,14 +535,14 @@ void  vhdl_block_netlist(FILE *fd, int i)
 
   dbg(1, "vhdl_block_netlist():       used components\n");
   /* print all components */
-  if(!vhdl_stop)
+  if(!vhdl_stop) {
     for(j=0;j<xctx->symbols;j++)
     {
       if( strcmp(get_tok_value(xctx->sym[j].prop_ptr,"vhdl_primitive",0),"true")==0 ) continue;
       if(!xctx->sym[j].type || (strcmp(xctx->sym[j].type,"primitive")!=0 && 
          strcmp(xctx->sym[j].type,"subcircuit")!=0))
            continue;
-      my_strdup(1238, &abs_path, abs_sym_path(xctx->sym[i].name, ""));
+      my_strdup2(1238, &abs_path, abs_sym_path(xctx->sym[i].name, ""));
       if(( strcmp(xctx->sym[j].type,"subcircuit")==0 || strcmp(xctx->sym[j].type,"primitive")==0) && 
           check_lib(1, abs_path)
         ) {
@@ -591,8 +591,9 @@ void  vhdl_block_netlist(FILE *fd, int i)
         if(tmp) fprintf(fd, "\n);\n");
         fprintf(fd, "end component ;\n\n");
       }
-      my_free(1239, &abs_path);
-    }
+    } /* for(j...) */
+  } /* if(!vhdl_stop) */
+  my_free(1239, &abs_path);
   dbg(1, "vhdl_block_netlist():  netlisting %s\n", skip_dir( xctx->sch[xctx->currsch]));
   vhdl_netlist(fd, vhdl_stop);
   fprintf(fd,"//// begin user architecture code\n");