From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sat, 16 Apr 2022 04:04:52 +0000 (-0700)
Subject: gvpack: rewrite 'xName' to return a 'std::string'
X-Git-Tag: 4.0.0~67^2~8
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8d045a2c0f3fa3452fa6a6e02aa4ca801bc90097;p=graphviz

gvpack: rewrite 'xName' to return a 'std::string'

This removes some manual memory management leaving an easier to read function,
as well as removing a static unfreed buffer that impedes Valgrind and ASan.
---

diff --git a/cmd/tools/gvpack.cpp b/cmd/tools/gvpack.cpp
index f52334135..15d37b206 100644
--- a/cmd/tools/gvpack.cpp
+++ b/cmd/tools/gvpack.cpp
@@ -27,6 +27,7 @@
 #include <ingraphs/ingraphs.h>
 #include <pack/pack.h>
 #include <stddef.h>
+#include <string>
 #include <vector>
 
 extern "C" {
@@ -507,32 +508,18 @@ static void cloneGraphAttr(Agraph_t * g, Agraph_t * ng)
  * create a new name using the old name and a number.
  * Note that returned string will immediately made into an agstring.
  */
-static char *xName(Dt_t * names, char *oldname)
-{
-    static char* name = NULL;
-    static size_t namelen = 0;
-    char *namep;
-    pair_t *p;
-    size_t len;
-
-    p = (pair_t*)dtmatch(names, oldname);
-    if (p) {
-	p->cnt++;
-	len = strlen(oldname) + 100; /* 100 for "_gv" and decimal no. */
-	if (namelen < len) {
-	    free (name);
-	    name = N_NEW(len, char);
-	    namelen = len;
-	}
-	sprintf(name, "%s_gv%d", oldname, p->cnt);
-	namep = name;
-    } else {
-	p = NEW(pair_t);
-	p->name = oldname;
-	dtinsert(names, p);
-	namep = oldname;
-    }
-    return namep;
+static std::string xName(Dt_t *names, char *oldname) {
+  auto p = reinterpret_cast<pair_t *>(dtmatch(names, oldname));
+  if (p) {
+    p->cnt++;
+    return std::string(oldname) + "_gv" + std::to_string(p->cnt);
+  }
+
+  p = NEW(pair_t);
+  p->name = oldname;
+  dtinsert(names, p);
+
+  return oldname;
 }
 
 #define MARK(e) (ED_alg(e) = e)
@@ -563,7 +550,8 @@ cloneSubg(Agraph_t * g, Agraph_t * ng, Agsym_t * G_bb, Dt_t * gnames)
 
     /* clone subgraphs */
     for (subg = agfstsubg (g); subg; subg = agnxtsubg (subg)) {
-	nsubg = agsubg(ng, xName(gnames, agnameof(subg)), 1);
+	nsubg = agsubg(ng, const_cast<char*>(xName(gnames, agnameof(subg)).c_str()),
+	               1);
 	agbindrec (nsubg, "Agraphinfo_t", sizeof(Agraphinfo_t), true);
 	cloneSubg(subg, nsubg, G_bb, gnames);
 	/* if subgraphs are clusters, point to the new 
@@ -686,14 +674,16 @@ static Agraph_t *cloneGraph(Agraph_t ** gs, int cnt, GVC_t * gvc)
 		fprintf(stderr, "Some nodes will be renamed.\n");
 		doWarn = false;
 	    }
-	    np = agnode(root, xName(nnames, agnameof(n)), 1);
+	    np = agnode(root, const_cast<char*>(xName(nnames, agnameof(n)).c_str()),
+	                1);
 	    agbindrec (np, "Agnodeinfo_t", sizeof(Agnodeinfo_t), true);
 	    ND_alg(n) = np;
 	    cloneNode(n, np);
 	}
 
 	/* wrap the clone of g in a subgraph of root */
-	subg = agsubg(root, xName(gnames, agnameof(g)), 1);
+	subg = agsubg(root, const_cast<char*>(xName(gnames, agnameof(g)).c_str()),
+	              1);
 	agbindrec (subg, "Agraphinfo_t", sizeof(Agraphinfo_t), true);
 	cloneSubg(g, subg, G_bb, gnames);
     }