diff --git a/src/bufio.h b/src/bufio.h index cdba842..23f90d5 100644 --- a/src/bufio.h +++ b/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; } diff --git a/test/bufio.carp b/test/bufio.carp index edef523..845e026 100644 --- a/test/bufio.carp +++ b/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")) diff --git a/test/mock_stream.h b/test/mock_stream.h index 4e09d53..d995c98 100644 --- a/test/mock_stream.h +++ b/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;