http.c: Unify read operations

So far, libweb would perform different read operations depending on its
state:

- For HTTP headers or request bodies, one byte at a time was read.
- For multipart/form-data, up to BUFSIZ bytes at a time were read.

However, this caused a significant extra number of syscalls for no
reason and would increase code complexity, specially when parsing
multiform/form-data boundaries.

Now, http_read always reads up to BUFSIZ bytes at a time and process
them on a loop. Apart from reducing code complexity, this should
increase performance due to the (much) lower number of syscalls
required.

1 files changed, 178 insertions, 159 deletions

diff --git a/http.c b/http.c
index 2026a8e..99677e9 100644
--- a/http.c
+++ b/http.c
@@ -76,7 +76,7 @@ struct http_ctx
                 off_t len, written;
                 char *boundary;
                 size_t blen, nforms, nfiles, npairs;
-                int fd;
+                FILE *f;
                 struct http_post_file *files;
                 struct http_post_pair *pairs;
 
@@ -89,8 +89,7 @@ struct http_ctx
             struct put
             {
                 char *tmpname;
-                int fd;
-                off_t written;
+                FILE *f;
             } put;
         } u;
 
@@ -469,8 +468,8 @@ static void ctx_free(struct ctx *const c)
         free(m->pairs);
         free(m->boundary);
 
-        if (m->fd >= 0 && close(m->fd))
-            fprintf(stderr, "%s: close(2) m->fd: %s\n",
+        if (m->f && fclose(m->f))
+            fprintf(stderr, "%s: fclose(3) m->f: %s\n",
                 __func__, strerror(errno));
 
         for (size_t i = 0; i < m->nforms; i++)
@@ -494,8 +493,8 @@ static void ctx_free(struct ctx *const c)
     {
         struct put *const p = &c->u.put;
 
-        if (p->fd >= 0 && close(p->fd))
-            fprintf(stderr, "%s: close(2) p->fd: %s\n",
+        if (p->f >= 0 && fclose(p->f))
+            fprintf(stderr, "%s: fclose(3) p->f: %s\n",
                 __func__, strerror(errno));
 
         free(c->u.put.tmpname);
@@ -909,7 +908,7 @@ static int set_length(struct http_ctx *const h, const char *const len)
             /* Fall through. */
         case HTTP_OP_PUT:
             c->payload.len = value;
-            c->u.put = (const struct put){.fd = -1};
+            c->u.put = (const struct put){0};
             break;
 
         case HTTP_OP_GET:
@@ -998,7 +997,7 @@ static int set_content_type(struct http_ctx *const h, const char *const type)
     }
 
     c->boundary = b.str;
-    c->u.mf = (const struct multiform){.fd = -1};
+    c->u.mf = (const struct multiform){0};
     return 0;
 }
 
@@ -1604,63 +1603,82 @@ static char *get_tmp(const char *const tmpdir)
 
 static int generate_mf_file(struct http_ctx *const h)
 {
+    int ret = -1;
     struct multiform *const m = &h->ctx.u.mf;
     struct form *const f = &m->forms[m->nforms - 1];
+    int fd = -1;
+    FILE *fp;
+    char *const tmpname = get_tmp(h->cfg.tmpdir);
 
-    if (!(f->tmpname = get_tmp(h->cfg.tmpdir)))
+    if (!tmpname)
     {
         fprintf(stderr, "%s: get_tmp failed\n", __func__);
-        return -1;
+        goto end;
     }
-    else if ((m->fd = mkstemp(f->tmpname)) < 0)
+    else if ((fd = mkstemp(tmpname)) < 0)
     {
         fprintf(stderr, "%s: mkstemp(3): %s\n", __func__, strerror(errno));
-        return -1;
+        goto end;
+    }
+    else if (!(fp = fdopen(fd, "wb")))
+    {
+        fprintf(stderr, "%s: fdopen(3): %s\n", __func__, strerror(errno));
+        goto end;
     }
 
-    return 0;
+    f->tmpname = tmpname;
+    m->f = fp;
+    ret = 0;
+
+end:
+    if (ret)
+    {
+        if (fd >= 0 && close(fd))
+            fprintf(stderr, "%s: close(2): %s\n", __func__, strerror(errno));
+
+        free(tmpname);
+    }
+
+    return ret;
 }
 
-static int read_mf_body_to_mem(struct http_ctx *const h, const void *const buf,
-    const size_t n)
+static int read_mf_body_to_mem(struct http_ctx *const h, const char b)
 {
     struct ctx *const c = &h->ctx;
     struct multiform *const m = &c->u.mf;
 
-    if (m->written + n > sizeof h->line)
+    if (m->written + 1 > sizeof h->line - 1)
     {
         fprintf(stderr, "%s: maximum length exceeded\n", __func__);
         return 1;
     }
 
-    memcpy(&h->line[m->written], buf, n);
-    m->written += n;
-    m->len += n;
-    c->payload.read += n;
+    h->line[m->written++] = b;
+    m->len++;
+    c->payload.read++;
     return 0;
 }
 
-static int read_mf_body_to_file(struct http_ctx *const h, const void *const buf,
-    const size_t n)
+static int read_mf_body_to_file(struct http_ctx *const h, const char b)
 {
     struct ctx *const c = &h->ctx;
     struct multiform *const m = &c->u.mf;
-    ssize_t res;
 
-    if (m->fd < 0 && generate_mf_file(h))
+    if (!m->f && generate_mf_file(h))
     {
         fprintf(stderr, "%s: generate_mf_file failed\n", __func__);
         return -1;
     }
-    else if ((res = pwrite(m->fd, buf, n, m->written)) < 0)
+    else if (!fwrite(&b, sizeof b, 1, m->f))
     {
-        fprintf(stderr, "%s: pwrite(2): %s\n", __func__, strerror(errno));
+        fprintf(stderr, "%s: fwrite(3) failed, feof=%d, ferror=%d\n",
+            __func__, feof(m->f), ferror(m->f));
         return -1;
     }
 
-    m->written += res;
-    m->len += res;
-    c->payload.read += res;
+    m->written++;
+    m->len++;
+    c->payload.read++;
     return 0;
 }
 
@@ -1668,41 +1686,44 @@ static int reset_boundary(struct http_ctx *const h)
 {
     struct multiform *const m = &h->ctx.u.mf;
     struct form *const f = &m->forms[m->nforms - 1];
-    int (*const read_mf)(struct http_ctx *, const void *, size_t) =
+    int (*const read_mf)(struct http_ctx *, char) =
             f->filename ? read_mf_body_to_file : read_mf_body_to_mem;
     const size_t len = strlen(m->boundary);
-    const int res = read_mf(h, m->boundary, len);
 
-    if (res)
-        return res;
+    for (size_t i = 0; i < len; i++)
+    {
+        const int res = read_mf(h, m->boundary[i]);
+
+        if (res)
+            return res;
+    }
 
     memset(m->boundary, '\0', len);
     m->blen = 0;
     return 0;
 }
 
-static int dump_body(struct http_ctx *const h, const void *const buf,
-    const size_t n)
+static int dump_body(struct http_ctx *const h, const char b)
 {
     struct multiform *const m = &h->ctx.u.mf;
     struct form *const f = &m->forms[m->nforms - 1];
-    int (*const read_mf)(struct http_ctx *, const void *, size_t) =
+    int (*const read_mf)(struct http_ctx *, char) =
             f->filename ? read_mf_body_to_file : read_mf_body_to_mem;
 
-    return read_mf(h, buf, n);
+    return read_mf(h, b);
 }
 
 static int apply_from_file(struct http_ctx *const h, struct form *const f)
 {
     struct multiform *const m = &h->ctx.u.mf;
 
-    if (close(m->fd))
+    if (fclose(m->f))
     {
-        fprintf(stderr, "%s: close(2): %s\n", __func__, strerror(errno));
+        fprintf(stderr, "%s: fclose(3): %s\n", __func__, strerror(errno));
         return -1;
     }
 
-    m->fd = -1;
+    m->f = NULL;
 
     const size_t n = m->nfiles + 1;
     struct http_post_file *const files = realloc(m->files,
@@ -1777,8 +1798,7 @@ static int apply_from_mem(struct http_ctx *const h, struct form *const f)
     return 0;
 }
 
-static int read_mf_body_boundary_byte(struct http_ctx *const h, const char b,
-    const size_t len)
+static int read_mf_body_boundary_byte(struct http_ctx *const h, const char b)
 {
     struct ctx *const c = &h->ctx;
     struct multiform *const m = &c->u.mf;
@@ -1845,117 +1865,55 @@ static const char *http_memmem(const char *const a, const void *const b,
     return st;
 }
 
-static int read_mf_body_boundary(struct http_ctx *const h,
-    const char **const buf, size_t *const n)
+static int read_mf_body_boundary(struct http_ctx *const h, const char b)
 {
-    const char *orig_buf = *buf;
-    const size_t orig_n = *n;
     struct ctx *const c = &h->ctx;
     struct multiform *const m = &c->u.mf;
     int res;
 
-    if (m->blen
-        && **buf != c->boundary[m->blen]
-        && (res = reset_boundary(h)))
-        return res;
-
-    const char *const boundary = http_memmem(&c->boundary[m->blen], *buf, *n);
-
-    if (!boundary)
+    if (b != c->boundary[m->blen])
     {
         if ((res = reset_boundary(h))
-            || (res = dump_body(h, *buf, *n)))
+            || (res = dump_body(h, b)))
             return res;
 
-        *n = 0;
         return 0;
     }
 
-    const size_t prev = boundary - *buf;
-
-    if (prev && (res = reset_boundary(h)))
-        return res;
-    else if ((res = dump_body(h, *buf, prev)))
-        return res;
-
-    *buf += prev;
-    *n -= prev;
-
-    const size_t len = strlen(m->boundary),
-        rem = strlen(c->boundary) - len,
-        r = rem > *n ? *n : rem;
-
-    for (size_t i = 0; i < r; i++)
-    {
-        const char *const b = *buf;
-
-        if ((res = read_mf_body_boundary_byte(h, b[i], len)))
-            return res;
-    }
-
-    *buf += r;
-    *n -= r;
-    return 0;
+    return read_mf_body_boundary_byte(h, b);
 }
 
-static int read_multiform_n(struct http_ctx *const h, bool *const close,
-    const char *buf, size_t n)
+static int read_multiform(struct http_ctx *const h, bool *const close,
+    const char b)
 {
     struct multiform *const m = &h->ctx.u.mf;
+    int res;
 
-    while (n)
+    switch (m->state)
     {
-        int res;
-
-        switch (m->state)
+        case MF_START_BOUNDARY:
+            /* Fall through. */
+        case MF_HEADER_CR_LINE:
+            /* Fall through. */
+        case MF_END_BOUNDARY_CR_LINE:
         {
-            case MF_START_BOUNDARY:
-                /* Fall through. */
-            case MF_HEADER_CR_LINE:
-                /* Fall through. */
-            case MF_END_BOUNDARY_CR_LINE:
-            {
-                if ((res = update_lstate(h, close, process_mf_line, *buf)))
-                    return res;
-
-                buf++;
-                n--;
-            }
+            if ((res = update_lstate(h, close, process_mf_line, b)))
+                return res;
+        }
 
-                break;
+            break;
 
-            case MF_BODY_BOUNDARY_LINE:
-                if ((res = read_mf_body_boundary(h, &buf, &n)))
-                    return res;
-        }
+        case MF_BODY_BOUNDARY_LINE:
+            if ((res = read_mf_body_boundary(h, b)))
+                return res;
     }
 
     return 0;
 }
 
-static int read_multiform(struct http_ctx *const h, bool *const close)
-{
-    /* Note: the larger the buffer below, the less CPU load. */
-    char buf[sizeof h->line];
-    struct payload *const p = &h->ctx.payload;
-    const unsigned long long left = p->len - p->read;
-    const size_t rem = left > sizeof buf ? sizeof buf : left;
-    const int r = h->cfg.read(buf, rem, h->cfg.user);
-
-    if (r <= 0)
-        return rw_error(r, close);
-
-    return read_multiform_n(h, close, buf, r);
-}
-
-static int read_body_to_mem(struct http_ctx *const h, bool *const close)
+static int read_body_to_mem(struct http_ctx *const h, bool *const close,
+    const char b)
 {
-    char b;
-    const int r = h->cfg.read(&b, sizeof b, h->cfg.user);
-
-    if (r <= 0)
-        return rw_error(r, close);
-
     struct ctx *const c = &h->ctx;
     struct payload *const p = &c->payload;
 
@@ -1992,45 +1950,95 @@ static int read_body_to_mem(struct http_ctx *const h, bool *const close)
     return 0;
 }
 
-static int read_put_body_to_file(struct http_ctx *const h,
-    const void *const buf, const size_t n)
+static int ensure_put_tmp(struct http_ctx *const h)
+{
+    int ret = -1;
+    struct put *const put = &h->ctx.u.put;
+    char *const tmpname = get_tmp(h->cfg.tmpdir);
+
+    if (!tmpname)
+    {
+        fprintf(stderr, "%s: get_tmp failed\n", __func__);
+        goto end;
+    }
+
+    put->tmpname = tmpname;
+    ret = 0;
+
+end:
+
+    if (ret)
+        free(tmpname);
+
+    return ret;
+}
+
+static int ensure_put_file(struct http_ctx *const h)
+{
+    int ret = -1;
+    struct put *const put = &h->ctx.u.put;
+    int fd = -1;
+    FILE *f = NULL;
+
+    if ((fd = mkstemp(put->tmpname)) < 0)
+    {
+        fprintf(stderr, "%s: mkstemp(3): %s\n", __func__, strerror(errno));
+        goto end;
+    }
+    else if (!(f = fdopen(fd, "wb")))
+    {
+        fprintf(stderr, "%s: fdopen(3): %s\n", __func__, strerror(errno));
+        goto end;
+    }
+
+    put->f = f;
+    ret = 0;
+
+end:
+    if (ret)
+    {
+        if (f && fclose(f))
+            fprintf(stderr, "%s: fclose(3): %s\n", __func__, strerror(errno));
+        else if (fd >= 0 && close(fd))
+            fprintf(stderr, "%s: close(2): %s\n", __func__, strerror(errno));
+    }
+
+    return ret;
+}
+
+static int read_put_body_to_file(struct http_ctx *const h, const char b)
 {
     struct ctx *const c = &h->ctx;
     struct put *const put = &c->u.put;
-    ssize_t res;
 
-    if (!put->tmpname && !(put->tmpname = get_tmp(h->cfg.tmpdir)))
+    if (!put->tmpname && ensure_put_tmp(h))
     {
-        fprintf(stderr, "%s: get_tmp failed\n", __func__);
+        fprintf(stderr, "%s: ensure_put_tmp failed\n", __func__);
         return -1;
     }
-    else if (put->fd < 0 && (put->fd = mkstemp(put->tmpname)) < 0)
+    else if (!put->f && ensure_put_file(h))
     {
-        fprintf(stderr, "%s: mkstemp(3): %s\n", __func__, strerror(errno));
+        fprintf(stderr, "%s: ensure_put_file failed\n", __func__);
         return -1;
     }
-    else if ((res = pwrite(put->fd, buf, n, c->payload.read)) < 0)
+    else if (!fwrite(&b, sizeof b, 1, put->f))
     {
-        fprintf(stderr, "%s: pwrite(2): %s\n", __func__, strerror(errno));
+        fprintf(stderr, "%s: fwrite(3) failed, feof=%d ferror=%d\n",
+            __func__, feof(put->f), ferror(put->f));
         return -1;
     }
 
-    c->payload.read += res;
+    c->payload.read++;
     return 0;
 }
 
-static int read_to_file(struct http_ctx *const h, bool *const close)
+static int read_to_file(struct http_ctx *const h, bool *const close,
+    const char b)
 {
-    char buf[BUFSIZ];
     struct ctx *const c = &h->ctx;
     struct payload *const p = &c->payload;
-    const unsigned long long left = p->len - p->read;
-    const size_t rem = left > sizeof buf ? sizeof buf : left;
-    const int r = h->cfg.read(buf, rem, h->cfg.user);
 
-    if (r <= 0)
-        return rw_error(r, close);
-    else if (read_put_body_to_file(h, buf, r))
+    if (read_put_body_to_file(h, b))
         return -1;
     else if (p->read >= p->len)
     {
@@ -2056,18 +2064,18 @@ static int read_to_file(struct http_ctx *const h, bool *const close)
     return 0;
 }
 
-static int read_body(struct http_ctx *const h, bool *const close)
+static int read_body(struct http_ctx *const h, bool *const close, const char b)
 {
     const struct ctx *const c = &h->ctx;
 
     switch (c->op)
     {
         case HTTP_OP_POST:
-            return c->boundary ? read_multiform(h, close)
-            : read_body_to_mem(h, close);
+            return c->boundary ? read_multiform(h, close, b)
+            : read_body_to_mem(h, close, b);
 
         case HTTP_OP_PUT:
-            return read_to_file(h, close);
+            return read_to_file(h, close, b);
 
         case HTTP_OP_GET:
             /* Fall through. */
@@ -2090,31 +2098,42 @@ static int process_line(struct http_ctx *const h)
     return state[h->ctx.state](h);
 }
 
-static int http_read(struct http_ctx *const h, bool *const close)
+static int read_byte(struct http_ctx *const h, bool *const close, const char b)
 {
     switch (h->ctx.state)
     {
         case START_LINE:
             /* Fall through. */
         case HEADER_CR_LINE:
-        {
-            char b;
-            const int r = h->cfg.read(&b, sizeof b, h->cfg.user);
-
-            if (r <= 0)
-                return rw_error(r, close);
-
             return update_lstate(h, close, process_line, b);
-        }
 
         case BODY_LINE:
-            return read_body(h, close);
+            return read_body(h, close, b);
     }
 
     fprintf(stderr, "%s: unexpected state %d\n", __func__, h->ctx.state);
     return -1;
 }
 
+int http_read(struct http_ctx *const h, bool *const close)
+{
+    char buf[BUFSIZ];
+    const int r = h->cfg.read(buf, sizeof buf, h->cfg.user);
+
+    if (r <= 0)
+        return rw_error(r, close);
+
+    for (int i = 0; i < r; i++)
+    {
+        const int ret = read_byte(h, close, buf[i]);
+
+        if (ret)
+            return ret;
+    }
+
+    return 0;
+}
+
 static int append_expire(struct dynstr *const d)
 {
     time_t t = time(NULL);