From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Tue, 5 Apr 2022 14:45:40 +0000 (-0700)
Subject: gc: replace inline stack implementation with generic one
X-Git-Tag: 4.0.0~113^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4e2875fd7376338259dcb3ccc8f029d58bdf22dd;p=graphviz

gc: replace inline stack implementation with generic one

This code was using two abstractions, a block `blk_t` and stack `stk_t`, to
amortize the cost of allocations. We can remove the block abstraction and
rewrite the stack implementation to use the simpler generic stack while still
retaining these amortization benefits. Note that this refactoring also makes
initialization of the stack data structure unnecessary as a zeroed `gv_stack_t`
is also a valid empty stack.

The new code also deallocates the stack prior to exit, aiding tools like
Valgrind and Address Sanitizer.

This pattern of using both a hand-rolled block and hand-rolled stack appears in
numerous places in the Graphviz code base, of which this is just one instance.

Gitlab: #1793
---

diff --git a/cmd/tools/gc.c b/cmd/tools/gc.c
index 863ae9b7e..459a3621e 100644
--- a/cmd/tools/gc.c
+++ b/cmd/tools/gc.c
@@ -17,12 +17,10 @@
 #include <stdlib.h>
 #include <string.h>
 
-#define NEW(t)           malloc(sizeof(t))
-#define N_NEW(n,t)       calloc((n),sizeof(t))
-
 #include <cgraph/cgraph.h>
 #include <cgraph/cghdr.h>
 #include <cgraph/exit.h>
+#include <cgraph/stack.h>
 
 typedef struct {
     Agrec_t h;
@@ -143,71 +141,20 @@ static void init(int argc, char *argv[])
     outfile = stdout;
 }
 
-typedef struct blk_t {
-    Agnode_t **data;
-    Agnode_t **endp;
-    struct blk_t *prev;
-    struct blk_t *next;
-} blk_t;
-
-typedef struct {
-    blk_t *fstblk;
-    blk_t *curblk;
-    Agnode_t **curp;
-} stk_t;
-
-
-#define SMALLBUF 1024
-#define BIGBUF 1000000
-
-static Agnode_t *base[SMALLBUF];
-static blk_t Blk;
-static stk_t Stk;
-
-static void initStk(void)
-{
-    Blk.data = base;
-    Blk.endp = Blk.data + SMALLBUF;
-    Stk.curblk = Stk.fstblk = &Blk;
-    Stk.curp = Stk.curblk->data;
-}
+static gv_stack_t Stk;
 
 static void push(Agnode_t * np)
 {
-    if (Stk.curp == Stk.curblk->endp) {
-	if (Stk.curblk->next == NULL) {
-	    blk_t *bp = NEW(blk_t);
-	    if (bp == 0) {
-		fprintf(stderr, "gc: Out of memory\n");
-		graphviz_exit(1);
-	    }
-	    bp->prev = Stk.curblk;
-	    bp->next = NULL;
-	    bp->data = N_NEW(BIGBUF, Agnode_t *);
-	    if (bp->data == 0) {
-		fprintf(stderr, "gc: Out of memory\n");
-		graphviz_exit(1);
-	    }
-	    bp->endp = bp->data + BIGBUF;
-	    Stk.curblk->next = bp;
-	}
-	Stk.curblk = Stk.curblk->next;
-	Stk.curp = Stk.curblk->data;
-    }
     ND_dfs_mark(np) = -1;
-    *Stk.curp++ = np;
+    stack_push_or_exit(&Stk, np);
 }
 
 static Agnode_t *pop(void)
 {
-    if (Stk.curp == Stk.curblk->data) {
-	if (Stk.curblk == Stk.fstblk)
-	    return 0;
-	Stk.curblk = Stk.curblk->prev;
-	Stk.curp = Stk.curblk->endp;
+    if (stack_is_empty(&Stk)) {
+	return 0;
     }
-    Stk.curp--;
-    return *Stk.curp;
+    return stack_pop(&Stk);
 }
 
 static void cc_dfs(Agraph_t * g, Agnode_t * n)
@@ -355,9 +302,6 @@ int main(int argc, char *argv[])
     init(argc, argv);
     newIngraph(&ig, Files, gread);
 
-    if (flags & CC)
-	initStk();
-
     while ((g = nextGraph(&ig)) != 0) {
 	if (prev)
 	    agclose(prev);
@@ -372,5 +316,7 @@ int main(int argc, char *argv[])
     if (n_graphs > 1)
 	wcp(tot_nodes, tot_edges, tot_cc, tot_cl, "total", 0);
 
+    stack_reset(&Stk);
+
     graphviz_exit(rv);
 }