Reject a count below one in read-n instead of crashing

bufio.carp

@@ -106,14 +106,18 @@ Ends and fails exactly like `read-line`, which is this function with `\\n`.")
 A stream that ends early yields the bytes it did produce; if that is none the
 result is `Error \"connection closed\"`. A stream that fails yields
 `Error \"read error\"` and the bytes already copied out are gone — unlike
-`read-line`, a partial `read-n` cannot be resumed.")
+`read-line`, a partial `read-n` cannot be resumed.
+
+An `n` below `1` is `Error \"invalid count\"` and reads nothing.")
   (defn read-n [br n]
-    (let [status 0
-          arr (read-n- br n &status)]
-      (cond
-        (< status 0) (Result.Error @"read error")
-        (Array.empty? &arr) (Result.Error @"connection closed")
-        (Result.Success arr))))
+    (if (< n 1)
+      (Result.Error @"invalid count")
+      (let [status 0
+            arr (read-n- br n &status)]
+        (cond
+          (< status 0) (Result.Error @"read error")
+          (Array.empty? &arr) (Result.Error @"connection closed")
+          (Result.Success arr)))))
 
   (doc read-append "reads available data and appends to the byte buffer.
 Returns `(Result Int String)` with the number of bytes read; `0` means the

docs/BufReader.html

@@ -218,6 +218,7 @@ <h3 id="read-n">
 result is <code>Error &quot;connection closed&quot;</code>. A stream that fails yields
 <code>Error &quot;read error&quot;</code> and the bytes already copied out are gone — unlike
 <code>read-line</code>, a partial <code>read-n</code> cannot be resumed.</p>
+<p>An <code>n</code> below <code>1</code> is <code>Error &quot;invalid count&quot;</code> and reads nothing.</p>
 
                     </p>
                 </div>

src/bufio.h

@@ -161,15 +161,23 @@ String BufReader_read_MINUS_line_(BufReader* br, int* status) {
 
 Array BufReader_read_MINUS_n_(BufReader* br, int n, int* status) {
   Array result;
-  result.data = CARP_MALLOC(n);
-  result.capacity = n;
+  result.data = NULL;
+  result.capacity = 0;
   result.len = 0;
   *status = BUFIO_OK;
+  if (n <= 0) return result;
 
-  while (result.len < n) {
+  result.data = CARP_MALLOC((size_t)n);
+  if (!result.data) {
+    *status = BUFIO_ERR;
+    return result;
+  }
+  result.capacity = (size_t)n;
+
+  while (result.len < (size_t)n) {
     int avail = bufreader_available(br);
     if (avail > 0) {
-      int want = n - result.len;
+      int want = n - (int)result.len;
       int take = avail < want ? avail : want;
       memcpy((char*)result.data + result.len, br->rbuf + br->rbuf_pos, take);
       result.len += take;

test/bufio.carp

@@ -10,6 +10,7 @@
 (register mock-buffered-write-len
   (Fn [&BufReader] Int)
   "mock_buffered_write_len")
+(register mock-read-n-raw (Fn [&BufReader Int] Int) "mock_read_n_raw")
 (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")
@@ -167,6 +168,54 @@ world
       (match r (Result.Success _) @"UNEXPECTED" (Result.Error e) e))
     "read-n returns error on empty stream")
 
+  (assert-equal test
+    "invalid count"
+    &(let-do [br (mock-bufreader-create "hello world" 0)
+              r (BufReader.read-n &br -1)]
+      (BufReader.delete br)
+      (mock-cleanup)
+      (match r (Result.Success _) @"UNEXPECTED" (Result.Error e) e))
+    "read-n rejects a negative count instead of crashing")
+
+  (assert-equal test
+    "invalid count"
+    &(let-do [br (mock-bufreader-create "hello world" 0)
+              r (BufReader.read-n &br 0)]
+      (BufReader.delete br)
+      (mock-cleanup)
+      (match r (Result.Success _) @"UNEXPECTED" (Result.Error e) e))
+    "read-n rejects a count of zero")
+
+  (assert-equal test
+    "hello"
+    &(let-do [br (mock-bufreader-create "hello world" 0)
+              _rejected (BufReader.read-n &br -1)
+              r (BufReader.read-n &br 5)]
+      (BufReader.delete br)
+      (mock-cleanup)
+      (match r
+        (Result.Success arr) (mock-bytes-to-string &arr)
+        (Result.Error _) @"ERROR"))
+    "a rejected count leaves the stream where it was")
+
+  (assert-equal test
+    0
+    (let-do [br (mock-bufreader-create "hello world" 0)
+             n (mock-read-n-raw &br -1)]
+      (BufReader.delete br)
+      (mock-cleanup)
+      n)
+    "the C entry point reads nothing for a negative count")
+
+  (assert-equal test
+    5
+    (let-do [br (mock-bufreader-create "hello world" 0)
+             n (mock-read-n-raw &br 5)]
+      (BufReader.delete br)
+      (mock-cleanup)
+      n)
+    "the C entry point still reads a positive count")
+
   (assert-equal test
     5
     (let-do [br (mock-bufreader-create "hello" 0)

test/mock_stream.h

@@ -104,6 +104,14 @@ static void mock_set_read_limits(int budget, int fail_code) {
 
 static int mock_buffered_write_len(BufReader* br) { return br->wbuf_len; }
 
+/* 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;
+  Array a = BufReader_read_MINUS_n_(br, n, &status);
+  if (a.data) CARP_FREE(a.data);
+  return status < 0 ? -1 : (int)a.len;
+}
+
 /* Long-typed view of bufio_next_cap, which takes and returns size_t; -1 for an
    argument that not every size_t can represent. */
 static int64_t bufio_next_cap_long(int64_t have, int64_t need) {