cgraph: rewrite scanner to use an agxbuf

In a sort of Chesterton’s fence¹ sense, I do not understand the existing code.
While I understand the first order motivation (dynamic string buffering inline),
I do not follow the implementation. It manages to hit the trifecta:

  1. NULL pointer dereference. Failures of allocation functions were not
     checked.

  2. Memory leaks. Calls to `agstrdup` and `agstrdup_html` allocate an owned
     string internally, yet this code was acting as if they stole the `Sbuf`
     backing memory.

  3. Not-obviously-correct and repeated computation of the end of the buffer to
     append to. `addstr` seemed overly complex and inefficient.

Static initialization of an agxbuf was a bit complicated until
04a248348798a4a32fc9ac7e5d860a84ab5ee49f, though it was still possible.
Following that commit, it becomes a no-brainer.

The code after this change still leaks memory. Perhaps in future we should reset
(`agxbfree(&Sbuf); memset(&Sbuf, 0, sizeof(Sbuf));`) the buffer after `agstrdup`
or `agstrdup_html`, though there would be a performance cost to this.

¹ https://en.wikipedia.org/wiki/G._K._Chesterton#Chesterton's_fence

diff --git a/lib/cgraph/scan.l b/lib/cgraph/scan.l
index a668cf75a0067a830f84b4a04e3974e270eed712..db6b6be882343553ad37cc7b0ea353ca0677dfb3 100644 (file)
--- a/lib/cgraph/scan.l
+++ b/lib/cgraph/scan.l
@@ -23,9 +23,11 @@
 %option noinput
 
 %{
+#include <assert.h>
 #include <grammar.h>
 #include <cghdr.h>
 #include <agxbuf.h>
+#include <cgraph/agxbuf.h>
 #include <ctype.h>
 #include <stdbool.h>
 #include <stddef.h>
@@ -68,40 +70,25 @@ int gv_isatty_suppression;
 #endif
 
 /* buffer for arbitrary length strings (longer than BUFSIZ) */
-static char    *Sbuf,*Sptr,*Send;
+static agxbuf Sbuf;
+
 static void beginstr(void) {
-       if (Sbuf == NULL) {
-               Sbuf = malloc(BUFSIZ);
-               Send = Sbuf + BUFSIZ;
-       }
-       Sptr = Sbuf;
-       *Sptr = 0;
+  // nothing required, but we should not have pending string data
+  assert(agxblen(&Sbuf) == 0 &&
+         "pending string data that was not consumed (missing "
+         "endstr()/endhtmlstr()?)");
 }
 
 static void addstr(char *src) {
-       char    c;
-       if (Sptr > Sbuf) Sptr--;
-       do {
-               do {c = *Sptr++ = *src++;} while (c && Sptr < Send);
-               if (c) {
-                       long    sz = Send - Sbuf;
-                       long    off = Sptr - Sbuf;
-                       sz *= 2;
-                       Sbuf = realloc(Sbuf,sz);
-                       Send = Sbuf + sz;
-                       Sptr = Sbuf + off;
-               }
-       } while (c);
+  agxbput(&Sbuf, src);
 }
 
 static void endstr(void) {
-       aaglval.str = agstrdup(Ag_G_global,Sbuf);
-       *Sbuf = 0;
+  aaglval.str = agstrdup(Ag_G_global, agxbuse(&Sbuf));
 }
 
 static void endstr_html(void) {
-       aaglval.str = agstrdup_html(Ag_G_global,Sbuf);
-       *Sbuf = 0;
+  aaglval.str = agstrdup_html(Ag_G_global, agxbuse(&Sbuf));
 }
 
 static void
@@ -247,20 +234,20 @@ void aagerror(const char *str)
        else switch (YYSTATE) {
        case qstring :
                agxbprint(&xb, " scanning a quoted string (missing endquote? longer than %d?)", YY_BUF_SIZE);
-               if (*Sbuf) {
-                       size_t len = strlen(Sbuf);
+               if (agxblen(&Sbuf) > 0) {
+                       size_t len = agxblen(&Sbuf);
                        if (len > 80)
-                               Sbuf[80] = '\0';
-                       agxbprint (&xb, "\nString starting:\"%s", Sbuf);
+                               len = 80;
+                       agxbprint(&xb, "\nString starting:\"%.*s", (int)len, Sbuf.buf);
                }
                break;
        case hstring :
                agxbprint(&xb, " scanning a HTML string (missing '>'? bad nesting? longer than %d?)", YY_BUF_SIZE);
-               if (*Sbuf) {
-                       size_t len = strlen(Sbuf);
+               if (agxblen(&Sbuf)) {
+                       size_t len = agxblen(&Sbuf);
                        if (len > 80)
-                               Sbuf[80] = '\0';
-                       agxbprint (&xb, "\nString starting:<%s", Sbuf);
+                               len = 80;
+                       agxbprint(&xb, "\nString starting:<%.*s", (int)len, Sbuf.buf);
                }
                break;
        case comment :