From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sat, 5 Feb 2022 06:05:42 +0000 (+1100)
Subject: gxl2gv: replace custom 'slist' string stack with generic stack
X-Git-Tag: 4.0.0~68^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=be7e79aacb33cc0597ff0b3ccba651933387517f;p=graphviz

gxl2gv: replace custom 'slist' string stack with generic stack

Similar to previous changes to `gc` in 4e2875fd7376338259dcb3ccc8f029d58bdf22dd,
this replaces some duplicated functionality with the generic Graphviz stack
implementation. Apart from reducing code duplication and complexity, this
removes an unnecessary rounding of allocations. The prior code rounded up
`slist` allocations to `sizeof(void*)`, but `malloc` effectively does this
internally anyway. This also adds checks for allocation failures that were
previously missing.

Gitlab: #1793
---

diff --git a/cmd/tools/gxl2gv.c b/cmd/tools/gxl2gv.c
index 223b5c844..a1fbaaa1f 100644
--- a/cmd/tools/gxl2gv.c
+++ b/cmd/tools/gxl2gv.c
@@ -11,7 +11,9 @@
 #include    <assert.h>
 #include    "convert.h"
 #include    <cgraph/agxbuf.h>
+#include    <cgraph/alloc.h>
 #include    <cgraph/exit.h>
+#include    <cgraph/likely.h>
 #include    <cgraph/stack.h>
 #include    <common/memory.h>
 #ifdef HAVE_EXPAT
@@ -47,60 +49,49 @@ typedef enum {
   TAG_HTML_LIKE_STRING,
 } attr_t;
 
-typedef struct slist slist;
-struct slist {
-    slist *next;
-    char buf[1];
-};
+static void pushString(gv_stack_t *stk, const char *s) {
 
-/* Round x up to next multiple of y, which is a power of 2 */
-#define ROUND2(x,y) (((x) + ((y)-1)) & ~((y)-1))
+  // duplicate the string we will push
+  char *copy = gv_strdup(s);
 
-static void pushString(slist ** stk, const char *s)
-{
-    size_t sz = ROUND2(sizeof(slist) + strlen(s), sizeof(void *));
-    slist *sp = gcalloc(sz, sizeof(char));
-    strcpy(sp->buf, s);
-    sp->next = *stk;
-    *stk = sp;
+  // push this onto the stack
+  stack_push_or_exit(stk, copy);
 }
 
-static void popString(slist ** stk)
-{
-    slist *sp = *stk;
-    if (!sp) {
-	fprintf(stderr, "PANIC: gxl2gv: empty element stack\n");
-	graphviz_exit(1);
-    }
-    *stk = sp->next;
-    free(sp);
-}
+static void popString(gv_stack_t *stk) {
 
-static char *topString(slist * stk)
-{
-    if (!stk) {
-	fprintf(stderr, "PANIC: gxl2gv: empty element stack\n");
-	graphviz_exit(1);
-    }
-    return stk->buf;
+  if (UNLIKELY(stack_is_empty(stk))) {
+    fprintf(stderr, "PANIC: gxl2gv: empty element stack\n");
+    graphviz_exit(EXIT_FAILURE);
+  }
+
+  char *s = stack_pop(stk);
+  free(s);
 }
 
-static void freeString(slist * stk)
-{
-    slist *sp;
+static char *topString(gv_stack_t *stk) {
 
-    while (stk) {
-	sp = stk->next;
-	free(stk);
-	stk = sp;
-    }
+  if (UNLIKELY(stack_is_empty(stk))) {
+    fprintf(stderr, "PANIC: gxl2gv: empty element stack\n");
+    graphviz_exit(EXIT_FAILURE);
+  }
+
+  return stack_top(stk);
+}
+
+static void freeString(gv_stack_t *stk) {
+  while (!stack_is_empty(stk)) {
+    char *s = stack_pop(stk);
+    free(s);
+  }
+  stack_reset(stk);
 }
 
 typedef struct {
     agxbuf xml_attr_name;
     agxbuf xml_attr_value;
     agxbuf composite_buffer;
-    slist *elements;
+    gv_stack_t elements;
     int listen;
     attr_t closedElementType;
     attr_t globalAttrType;
@@ -162,7 +153,7 @@ static userdata_t *genUserdata(void)
     agxbinit(&(user->xml_attr_value), SMALLBUF, 0);
     agxbinit(&(user->composite_buffer), SMALLBUF, 0);
     user->listen = FALSE;
-    user->elements = 0;
+    user->elements = (gv_stack_t){0};
     user->closedElementType = TAG_NONE;
     user->globalAttrType = TAG_NONE;
     user->compositeReadState = FALSE;
@@ -177,7 +168,7 @@ static void freeUserdata(userdata_t * ud)
     agxbfree(&(ud->xml_attr_name));
     agxbfree(&(ud->xml_attr_value));
     agxbfree(&(ud->composite_buffer));
-    freeString(ud->elements);
+    freeString(&ud->elements);
     free(ud);
 }
 
@@ -609,7 +600,7 @@ static void endElementHandler(void *userData, const char *name)
 	popString(&ud->elements);
 	ud->closedElementType = TAG_GRAPH;
     } else if (strcmp(name, "node") == 0) {
-	char *ele_name = topString(ud->elements);
+	char *ele_name = topString(&ud->elements);
 	if (ud->closedElementType == TAG_GRAPH) {
 	    Agnode_t *node = agnode(root, ele_name, 0);
 	    agdelete(root, node);