From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sat, 30 Jul 2022 00:24:05 +0000 (-0700)
Subject: cgraph: when an agxbuf to be expanded has 0 size, expand by 'BUFSIZ'
X-Git-Tag: 5.0.1~16^2~3
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e2a961758f664bb40cc18878af047455363044f4;p=graphviz

cgraph: when an agxbuf to be expanded has 0 size, expand by 'BUFSIZ'

When calling `agxbmore` on a 0-sized buffer, the new size to expand to would be
chosen based on the length of the current string being appended. This string
often originated from user’s input graph and hence its length was often
user-defined. There is nothing strictly wrong with this, but it is a little
unpredictable and sometimes leads to pathological outcomes, e.g:

  agxbputc(&xb, 'a'); // expands 0-sized buffer to 1
  agxbputc(&xb, 'b'); // doubles 1-sized buffer to 2
  agxbputc(&xb, 'c'); // doubles 2-sized buffer to 4
  agxbputc(&xb, 'd');
  agxbputc(&xb, 'e'); // doubles 4-sized buffer to 8
  …

This is functionally correct, but that is a lot of `realloc` calls just to
accrue 5 bytes.¹ It is more convenient to enlarge our initial allocation to
something matching the default hint when an agxbuf is initialized.

The situation described above has not typically been a concern in the past
because a 0-sized agxbuf is hard to obtain to begin with. You need to either
call `agxbinit` while supplying your own backing storage that also has 0 size
(why would you do this?) or reuse an agxbuf after `agxbdisown` (technically
legal, but no good reason to do this). Neither of these scenarios occur anywhere
in the Graphviz tree to my knowledge. However an upcoming change will make
0-sized agxbufs more common.

¹ Some of these `realloc`s are likely no-ops, as the system allocator generally
  does not give you exactly 1 byte when you ask for 1. It will probably give you
  more and then when you ask to double to 2, it will tell you your existing
  pointer is just fine to keep on using. But the agxbuf code does not know any
  of these details and will still repeatedly call into `realloc`.
---

diff --git a/lib/cgraph/agxbuf.h b/lib/cgraph/agxbuf.h
index 0f1c1b294..12bf6c3f6 100644
--- a/lib/cgraph/agxbuf.h
+++ b/lib/cgraph/agxbuf.h
@@ -76,7 +76,7 @@ static inline void agxbmore(agxbuf *xb, size_t ssz) {
   char *nbuf;       // new buffer
 
   size = (size_t)(xb->eptr - xb->buf);
-  nsize = 2 * size;
+  nsize = size == 0 ? BUFSIZ : (2 * size);
   if (size + ssz > nsize)
     nsize = size + ssz;
   cnt = (size_t)(xb->ptr - xb->buf);