[PATCH 0/5] a few odds and ends before full backtrace support

[PATCH 0/5] a few odds and ends before full backtrace support

[Eric Wong]

Eric Wong (5):
  httpd: hoist out err_close from ferror|fclose
  httpd: consolidate open_memstream error checking
  use `%m' for *printf rather than strerror(errno)
  httpd: mw_h1req comment fixes
  mwrap_httpd: report ftello error

 mwrap_core.h  |  8 +++---
 mwrap_httpd.h | 67 ++++++++++++++++++++++++++-------------------------
 2 files changed, 37 insertions(+), 38 deletions(-)

[PATCH 1/5] httpd: hoist out err_close from ferror|fclose

[Eric Wong]

A small tweak to simplify error reporting if we have any
problems due to OOM or bugs.
---
 mwrap_httpd.h | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/mwrap_httpd.h b/mwrap_httpd.h
index 6856208..b55bb02 100644
--- a/mwrap_httpd.h
+++ b/mwrap_httpd.h
@@ -212,6 +212,13 @@ static FILE *wbuf_new(struct mw_membuf *mb)
 	return fp;
 }
 
+static int err_close(FILE *fp)
+{
+	int e = ferror(fp) | fclose(fp);
+	if (e) fprintf(stderr, "ferror|fclose: %m\n");
+	return e;
+}
+
 static enum mw_qev h1_res_oneshot(struct mw_h1 *h1, const char *buf, size_t len)
 {
 	struct mw_membuf mb;
@@ -220,10 +227,8 @@ static enum mw_qev h1_res_oneshot(struct mw_h1 *h1, const char *buf, size_t len)
 	if (!fp)
 		return h1_close(h1);
 	fwrite(buf, 1, len, fp);
-	if (ferror(fp) | fclose(fp)) {
-		fprintf(stderr, "ferror|fclose: %m\n");
+	if (err_close(fp))
 		return h1_close(h1);
-	}
 
 	/* fill in the zero padding we added at wbuf_new */
 	mwrap_assert(!h1->wbuf);
@@ -258,10 +263,8 @@ static enum mw_qev h1_200(struct mw_h1 *h1, FILE *fp, struct mw_membuf *mb)
 	fprintf(fp, "%zu", (size_t)clen);
 	FPUTS("\r\n\r\n", fp);
 
-	if (ferror(fp) | fclose(fp)) {
-		fprintf(stderr, "ferror|fclose: %m\n");
+	if (err_close(fp))
 		return h1_close(h1);
-	}
 
 	/* fill in the zero-padding we added at wbuf_new */
 	mwrap_assert(!h1->wbuf);
@@ -462,8 +465,7 @@ static struct h1_src_loc *accumulate(unsigned long min, size_t *hslc, FILE *lp)
 	rcu_read_unlock();
 
 	struct h1_src_loc *hslv;
-	if (ferror(fp) | fclose(fp)) {
-		fprintf(stderr, "ferror|fclose: %m\n");
+	if (err_close(fp)) {
 		hslv = NULL;
 	} else {
 		*hslc = mb.len / sizeof(*hslv);
@@ -489,10 +491,9 @@ static enum mw_qev each_at(struct mw_h1 *h1, struct mw_h1req *h1r)
 	FILE *lp = open_memstream(&lname.ptr, &lname.len);
 	if (!lp) return h1_close(h1);
 	if (write_loc_name(lp, l) < 0) return h1_close(h1);
-	if (ferror(lp) | fclose(lp)) {
-		fprintf(stderr, "ferror|fclose: %m\n");
+	if (err_close(lp))
 		return h1_close(h1);
-	}
+
 	struct mw_membuf mb;
 	FILE *fp = wbuf_new(&mb);
 	if (!fp) return h1_close(h1);
@@ -544,8 +545,7 @@ static enum mw_qev each_gt(struct mw_h1 *h1, struct mw_h1req *h1r,
 	if (!hslv)
 		return h1_close(h1);
 
-	if (ferror(lp) | fclose(lp)) {
-		fprintf(stderr, "ferror|fclose: %m\n");
+	if (err_close(lp)) {
 		free(hslv);
 		return h1_close(h1);
 	}
@@ -863,8 +863,7 @@ static struct pollfd *poll_detach(struct mw_h1d *h1d, nfds_t *nfds)
 
 	/* not sure how to best recover from ENOMEM errors in stdio */
 	if (h1d->pfp) {
-		if (ferror(h1d->pfp) | fclose(h1d->pfp)) {
-			fprintf(stderr, "ferror|fclose: %m\n");
+		if (err_close(h1d->pfp)) {
 			exit(EXIT_FAILURE);
 		} else {
 			mwrap_assert(h1d->pbuf.len % sizeof(*pfd) == 0);

[PATCH 2/5] httpd: consolidate open_memstream error checking

[Eric Wong]

This adds error reporting to a few places where I forgot to
print errno on open_memstream failures.
---
 mwrap_httpd.h | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/mwrap_httpd.h b/mwrap_httpd.h
index b55bb02..b0adf6a 100644
--- a/mwrap_httpd.h
+++ b/mwrap_httpd.h
@@ -201,14 +201,19 @@ static enum mw_qev h1_send_flush(struct mw_h1 *h1)
 	return h1->persist ? MW_QEV_RD : h1_close(h1);
 }
 
+static FILE *memstream_new(struct mw_membuf *mb)
+{
+	FILE *fp = open_memstream(&mb->ptr, &mb->len);
+	if (!fp) fprintf(stderr, "open_memstream: %m\n");
+	return fp;
+}
+
 static FILE *wbuf_new(struct mw_membuf *mb)
 {
 	static const struct mw_wbuf pad;
-	FILE *fp = open_memstream(&mb->ptr, &mb->len);
+	FILE *fp = memstream_new(mb);
 	if (fp) /* pad space is populated before h1_send_flush */
 		fwrite(&pad, 1, sizeof(pad), fp);
-	else
-		fprintf(stderr, "open_memstream: %m\n");
 	return fp;
 }
 
@@ -432,11 +437,8 @@ static off_t write_loc_name(FILE *fp, const struct src_loc *l)
 static struct h1_src_loc *accumulate(unsigned long min, size_t *hslc, FILE *lp)
 {
 	struct mw_membuf mb;
-	FILE *fp = open_memstream(&mb.ptr, &mb.len);
-	if (!fp) {
-		fprintf(stderr, "open_memstream: %m\n");
-		return NULL;
-	}
+	FILE *fp = memstream_new(&mb);
+	if (!fp) return NULL;
 	rcu_read_lock();
 	struct cds_lfht *t = CMM_LOAD_SHARED(totals);
 	struct cds_lfht_iter iter;
@@ -488,7 +490,7 @@ static enum mw_qev each_at(struct mw_h1 *h1, struct mw_h1req *h1r)
 	if (!l) return h1_404(h1);
 
 	struct mw_membuf lname;
-	FILE *lp = open_memstream(&lname.ptr, &lname.len);
+	FILE *lp = memstream_new(&lname);
 	if (!lp) return h1_close(h1);
 	if (write_loc_name(lp, l) < 0) return h1_close(h1);
 	if (err_close(lp))
@@ -540,7 +542,8 @@ static enum mw_qev each_gt(struct mw_h1 *h1, struct mw_h1req *h1r,
 
 	size_t hslc;
 	struct mw_membuf ln;
-	FILE *lp = open_memstream(&ln.ptr, &ln.len);
+	FILE *lp = memstream_new(&ln);
+	if (!lp) return h1_close(h1);
 	struct h1_src_loc *hslv = accumulate(min, &hslc, lp);
 	if (!hslv)
 		return h1_close(h1);
@@ -845,11 +848,8 @@ static int poll_add(struct mw_h1d *h1d, int fd, short events)
 	struct pollfd pfd;
 
 	if (!h1d->pfp) {
-		h1d->pfp = open_memstream(&h1d->pbuf.ptr, &h1d->pbuf.len);
-		if (!h1d->pfp) {
-			fprintf(stderr, "open_memstream: %m\n");
-			return -1;
-		}
+		h1d->pfp = memstream_new(&h1d->pbuf);
+		if (!h1d->pfp) return -1;
 	}
 	pfd.fd = fd;
 	pfd.events = events;

[PATCH 3/5] use `%m' for *printf rather than strerror(errno)

[Eric Wong]

FreeBSD, glibc both support it, as do minimalist libcs such as
uClibc and musl.
---
 mwrap_core.h | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/mwrap_core.h b/mwrap_core.h
index 6d498c1..179f136 100644
--- a/mwrap_core.h
+++ b/mwrap_core.h
@@ -826,8 +826,7 @@ static void dump_destructor(void)
 		dump_fd = open(dump_path, O_CLOEXEC|O_WRONLY|O_APPEND|O_CREAT,
 				0666);
 		if (dump_fd < 0) {
-			fprintf(stderr, "open %s failed: %s\n", dump_path,
-				strerror(errno));
+			fprintf(stderr, "open %s failed: %m\n", dump_path);
 			goto out;
 		}
 	}
@@ -850,8 +849,7 @@ static void dump_destructor(void)
 			a.fp = fdopen(dump_fd, modes[i]);
 
 		if (!a.fp) {
-			fprintf(stderr, "failed to open fd=%d: %s\n",
-				dump_fd, strerror(errno));
+			fprintf(stderr, "failed to open fd=%d: %m\n", dump_fd);
 			goto out;
 		}
 		/* we'll leak some memory here, but this is a destructor */
@@ -948,7 +946,7 @@ __attribute__((constructor)) static void mwrap_ctor(void)
 		h->real = h;
 		call_rcu(&h->as.dead, free_hdr_rcu);
 	} else
-		fprintf(stderr, "malloc failed: %s\n", strerror(errno));
+		fprintf(stderr, "malloc: %m\n");
 
 	h1d_start();
 	CHECK(int, 0, pthread_sigmask(SIG_SETMASK, &old, NULL));

[PATCH 4/5] httpd: mw_h1req comment fixes

[Eric Wong]

---
 mwrap_httpd.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mwrap_httpd.h b/mwrap_httpd.h
index b0adf6a..6d40d6c 100644
--- a/mwrap_httpd.h
+++ b/mwrap_httpd.h
@@ -53,11 +53,11 @@ struct mw_wbuf { /* for response headers + bodies */
 struct mw_h1req { /* HTTP/1.x request (TSD in common (fast) case) */
 	const char *method, *path, *qstr;
 	size_t method_len, path_len, qlen;
-	uint16_t rbuf_len;
+	uint16_t rbuf_len; /* capped by MW_RBUF_SIZE */
 	int pret, minor_ver;
 	size_t nr_hdr;
 	struct phr_header hdr[MW_NR_NAME];
-	char rbuf[MW_RBUF_SIZE]; /* read(2) in to thi */
+	char rbuf[MW_RBUF_SIZE]; /* read(2) in to this */
 };
 
 struct mw_h1 { /* each HTTP/1.x client (heap) */

[PATCH 5/5] mwrap_httpd: report ftello error

[Eric Wong]

ftello(3) may fail if out-of-memory.
---
 mwrap_httpd.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mwrap_httpd.h b/mwrap_httpd.h
index 6d40d6c..bfa723d 100644
--- a/mwrap_httpd.h
+++ b/mwrap_httpd.h
@@ -429,8 +429,10 @@ static off_t write_loc_name(FILE *fp, const struct src_loc *l)
 		free(s);
 	}
 	off_t end = ftello(fp);
-	if (end < 0)
+	if (end < 0) {
+		fprintf(stderr, "ftello: %m\n");
 		return end;
+	}
 	return end - beg;
 }