Check every fixed-size allocation instead of dereferencing NULL

src/bufio.h

@@ -33,6 +33,8 @@ typedef struct {
 
 /* --- Construction / destruction --- */
 
+/* A buffer that could not be allocated is NULL with capacity 0, holding
+   nothing; bufio_reserve allocates it on first use. */
 BufReader BufReader_create_(void* inner, bufio_read_fn rfn,
                             bufio_write_fn wfn, bufio_close_fn cfn) {
   BufReader br;
@@ -43,10 +45,10 @@ BufReader BufReader_create_(void* inner, bufio_read_fn rfn,
   br.rbuf = CARP_MALLOC(BUFIO_DEFAULT_CAP);
   br.rbuf_len = 0;
   br.rbuf_pos = 0;
-  br.rbuf_cap = BUFIO_DEFAULT_CAP;
+  br.rbuf_cap = br.rbuf ? BUFIO_DEFAULT_CAP : 0;
   br.wbuf = CARP_MALLOC(BUFIO_DEFAULT_CAP);
   br.wbuf_len = 0;
-  br.wbuf_cap = BUFIO_DEFAULT_CAP;
+  br.wbuf_cap = br.wbuf ? BUFIO_DEFAULT_CAP : 0;
   return br;
 }
 
@@ -92,8 +94,27 @@ static int bufio_reserve_array(Array* buf, size_t extra) {
   return 0;
 }
 
-static String bufio_empty_string(void) {
+/* Copy of the `used` bytes of `src` with capacity `cap`, falling back to a
+   tight buffer and then to none; `*cap_out` is the capacity obtained. */
+static char* bufio_dup_buf(const char* src, int used, int cap, int* cap_out) {
+  char* buf = cap > 0 ? CARP_MALLOC((size_t)cap) : NULL;
+  *cap_out = buf ? cap : 0;
+  if (!buf && used > 0) {
+    buf = CARP_MALLOC((size_t)used);
+    *cap_out = buf ? used : 0;
+  }
+  if (buf && used > 0) memcpy(buf, src, (size_t)used);
+  return buf;
+}
+
+/* Empty string for a read that produced nothing; NULL with `*status` set to
+   BUFIO_ERR if even one byte cannot be had. */
+static String bufio_empty_string(int* status) {
   String s = CARP_MALLOC(1);
+  if (!s) {
+    *status = BUFIO_ERR;
+    return s;
+  }
   s[0] = '\0';
   return s;
 }
@@ -108,7 +129,13 @@ static int bufreader_fill(BufReader* br) {
     br->rbuf_len = remaining;
     br->rbuf_pos = 0;
   }
-  if (bufio_reserve(&br->rbuf, &br->rbuf_cap, (size_t)br->rbuf_len, 1) != 0)
+  /* A capacity below the default belongs to a reader recovering from a failed
+     allocation. */
+  size_t extra = br->rbuf_cap < BUFIO_DEFAULT_CAP
+                   ? (size_t)(BUFIO_DEFAULT_CAP - br->rbuf_len)
+                   : 1;
+  if (bufio_reserve(&br->rbuf, &br->rbuf_cap, (size_t)br->rbuf_len, extra) != 0
+      && bufio_reserve(&br->rbuf, &br->rbuf_cap, (size_t)br->rbuf_len, 1) != 0)
     return -1;
   int space = br->rbuf_cap - br->rbuf_len;
   int n = br->read_fn(br->inner, br->rbuf + br->rbuf_len, space);
@@ -128,6 +155,10 @@ String BufReader_read_MINUS_until_(BufReader* br, char delim, int* status) {
       if (br->rbuf[i] == delim) {
         int len = i - br->rbuf_pos + 1;
         String s = CARP_MALLOC(len + 1);
+        if (!s) {
+          *status = BUFIO_ERR;
+          return bufio_empty_string(status);
+        }
         memcpy(s, br->rbuf + br->rbuf_pos, len);
         s[len] = '\0';
         br->rbuf_pos += len;
@@ -138,19 +169,23 @@ String BufReader_read_MINUS_until_(BufReader* br, char delim, int* status) {
     int r = bufreader_fill(br);
     if (r < 0) {
       *status = BUFIO_ERR;
-      return bufio_empty_string();
+      return bufio_empty_string(status);
     }
     if (r == 0) {
       int avail = bufreader_available(br);
       *status = BUFIO_EOF;
       if (avail > 0) {
         String s = CARP_MALLOC(avail + 1);
+        if (!s) {
+          *status = BUFIO_ERR;
+          return bufio_empty_string(status);
+        }
         memcpy(s, br->rbuf + br->rbuf_pos, avail);
         s[avail] = '\0';
         br->rbuf_pos += avail;
         return s;
       }
-      return bufio_empty_string();
+      return bufio_empty_string(status);
     }
   }
 }
@@ -218,7 +253,7 @@ int BufReader_write_(BufReader* br, String* data) {
   size_t len = strlen(*data);
   if (bufio_reserve(&br->wbuf, &br->wbuf_cap, (size_t)br->wbuf_len, len) != 0)
     return -1;
-  memcpy(br->wbuf + br->wbuf_len, *data, len);
+  if (len > 0) memcpy(br->wbuf + br->wbuf_len, *data, len);
   br->wbuf_len += (int)len;
   return (int)len;
 }
@@ -227,7 +262,7 @@ int BufReader_write_MINUS_bytes_(BufReader* br, Array* data) {
   size_t len = data->len;
   if (bufio_reserve(&br->wbuf, &br->wbuf_cap, (size_t)br->wbuf_len, len) != 0)
     return -1;
-  memcpy(br->wbuf + br->wbuf_len, data->data, len);
+  if (len > 0) memcpy(br->wbuf + br->wbuf_len, data->data, len);
   br->wbuf_len += (int)len;
   return (int)len;
 }
@@ -261,15 +296,11 @@ BufReader BufReader_copy(BufReader* br) {
   c.read_fn = br->read_fn;
   c.write_fn = br->write_fn;
   c.close_fn = br->close_fn;
-  c.rbuf_cap = br->rbuf_cap;
-  c.rbuf_len = br->rbuf_len;
-  c.rbuf_pos = br->rbuf_pos;
-  c.rbuf = CARP_MALLOC(c.rbuf_cap);
-  memcpy(c.rbuf, br->rbuf, c.rbuf_len);
-  c.wbuf_cap = br->wbuf_cap;
-  c.wbuf_len = br->wbuf_len;
-  c.wbuf = CARP_MALLOC(c.wbuf_cap);
-  memcpy(c.wbuf, br->wbuf, c.wbuf_len);
+  c.rbuf = bufio_dup_buf(br->rbuf, br->rbuf_len, br->rbuf_cap, &c.rbuf_cap);
+  c.rbuf_len = c.rbuf ? br->rbuf_len : 0;
+  c.rbuf_pos = c.rbuf ? br->rbuf_pos : 0;
+  c.wbuf = bufio_dup_buf(br->wbuf, br->wbuf_len, br->wbuf_cap, &c.wbuf_cap);
+  c.wbuf_len = c.wbuf ? br->wbuf_len : 0;
   return c;
 }
 

test/bufio.carp

@@ -11,6 +11,8 @@
   (Fn [&BufReader] Int)
   "mock_buffered_write_len")
 (register mock-read-n-raw (Fn [&BufReader Int] Int) "mock_read_n_raw")
+(register mock-starve-buffers (Fn [&BufReader] ()) "mock_starve_buffers")
+(register mock-read-calls (Fn [] Int) "mock_read_calls")
 (register bufio-next-cap-long (Fn [Long Long] Long) "bufio_next_cap_long")
 (register mock-get-output (Fn [] String) "mock_get_output")
 (register mock-is-closed? (Fn [] Bool) "mock_is_closed")
@@ -442,4 +444,80 @@ world
   (assert-equal test
     -1l
     (bufio-next-cap-long 1073741824l -1l)
-    "a request that is not a size is refused"))
+    "a request that is not a size is refused")
+
+  (assert-equal test "hello
+" &(let-do [br (mock-bufreader-create "hello
+world
+" 0)] (mock-starve-buffers &br) (let-do [r (BufReader.read-line &br)]
+        (BufReader.delete br)
+        (mock-cleanup)
+        (match r (Result.Success s) s (Result.Error _) @"ERROR"))) "a reader whose read buffer could not be allocated still reads a line")
+
+  (assert-equal test
+    "hello world"
+    &(let-do [br (mock-bufreader-create "" 0)]
+      (mock-starve-buffers &br)
+      (ignore (BufReader.write &br "hello world"))
+      (ignore (BufReader.flush &br))
+      (let-do [output (mock-get-output)]
+        (BufReader.delete br)
+        (mock-cleanup)
+        output))
+    "a reader whose write buffer could not be allocated still writes")
+
+  (assert-true test
+    (let-do [payload (String.repeat 3000 "0123456789")
+             br (mock-bufreader-create &payload 0)]
+      (mock-starve-buffers &br)
+      (let-do [r (BufReader.read-until &br \;)]
+        (BufReader.delete br)
+        (mock-cleanup)
+        (match r (Result.Success s) (= &s &payload) (Result.Error _) false)))
+    "a reader with no read buffer grows back past the default capacity")
+
+  (assert-equal test "world
+" &(let-do [br (mock-bufreader-create "hello
+world
+" 0)
+            _first-line (BufReader.read-line &br)
+            c (BufReader.copy &br)
+            r (BufReader.read-line &c)] (BufReader.delete c) (BufReader.delete br) (mock-cleanup) (match r
+        (Result.Success s) s
+        (Result.Error _) @"ERROR")) "a copy keeps the bytes the original had buffered")
+
+  (assert-equal test "hello
+" &(let-do [br (mock-bufreader-create "hello
+world
+" 0)] (mock-starve-buffers &br) (let-do [c (BufReader.copy &br)
+                                         r (BufReader.read-line &c)]
+        (BufReader.delete c)
+        (BufReader.delete br)
+        (mock-cleanup)
+        (match r (Result.Success s) s (Result.Error _) @"ERROR"))) "a copy of a reader with no buffers allocates on first use")
+
+  (assert-equal test
+    1
+    (let-do [payload (String.repeat 400 "0123456789")
+             br (mock-bufreader-create &payload 0)]
+      (mock-starve-buffers &br)
+      (ignore (BufReader.read-n &br 4000))
+      (let-do [calls (mock-read-calls)]
+        (BufReader.delete br)
+        (mock-cleanup)
+        calls))
+    "a reader with no read buffer refills at the default capacity, not one byte per call")
+
+  (assert-equal test
+    "hi"
+    &(let-do [br (mock-bufreader-create "" 0)]
+      (mock-starve-buffers &br)
+      (ignore (BufReader.write &br ""))
+      (ignore (BufReader.write-bytes &br &(the (Array Byte) [])))
+      (ignore (BufReader.write &br "hi"))
+      (ignore (BufReader.flush &br))
+      (let-do [output (mock-get-output)]
+        (BufReader.delete br)
+        (mock-cleanup)
+        output))
+    "an empty write to a reader with no write buffer buffers nothing and leaves it usable"))

test/mock_stream.h

@@ -20,12 +20,14 @@ typedef struct {
   int write_fail_code;
   int read_budget;
   int read_fail_code;
+  int read_calls;
 } MockStream;
 
 static MockStream* g_mock = NULL;
 
 static int mock_stream_read(void* inner, char* buf, int len) {
   MockStream* ms = (MockStream*)inner;
+  ms->read_calls++;
   if (ms->read_budget >= 0) {
     if (ms->read_budget == 0) return ms->read_fail_code;
     if (len > ms->read_budget) len = ms->read_budget;
@@ -79,6 +81,7 @@ static BufReader mock_bufreader_create(String* data, int chunk_size) {
   ms->write_fail_code = -1;
   ms->read_budget = -1;
   ms->read_fail_code = -1;
+  ms->read_calls = 0;
   g_mock = ms;
   return BufReader_create_(
     (void*)ms, mock_stream_read, mock_stream_write, mock_stream_close);
@@ -104,6 +107,22 @@ static void mock_set_read_limits(int budget, int fail_code) {
 
 static int mock_buffered_write_len(BufReader* br) { return br->wbuf_len; }
 
+static int mock_read_calls(void) { return g_mock ? g_mock->read_calls : -1; }
+
+/* Puts a reader in the state BufReader_create_ leaves behind when neither
+   buffer could be allocated. */
+static void mock_starve_buffers(BufReader* br) {
+  if (br->rbuf) CARP_FREE(br->rbuf);
+  if (br->wbuf) CARP_FREE(br->wbuf);
+  br->rbuf = NULL;
+  br->rbuf_cap = 0;
+  br->rbuf_len = 0;
+  br->rbuf_pos = 0;
+  br->wbuf = NULL;
+  br->wbuf_cap = 0;
+  br->wbuf_len = 0;
+}
+
 /* read-n's C entry point unguarded by the Carp wrapper; -1 on error status. */
 static int mock_read_n_raw(BufReader* br, int n) {
   int status = BUFIO_OK;