util: one whole-file reader for all tools

Three private ones had grown: fnread() in util.c, flen() behind
pid_cmdline()/pid_cgroup() in cgutil.c, and conf_read_template() in
conf.c.  Two of them were also wrong in ways the others were not.

fnread() formatted the path into a char[256] and stat()ed it before
opening, so a longer path was silently truncated and then read from
whichever file the truncation happened to name, and the size could
change between the look and the read.  flen() existed because neither
of those approaches works on procfs at all, where stat() reports zero
and the only way to learn the size is to read to EOF.

Add fslurp() to util.[ch], which every tool already links.  It opens
first and sizes the fd it holds, treats st_size as a hint, and reads
until EOF, so procfs and regular files take the same path.  Paths are
formatted by libite's vfopenf(), which allocates to fit.  Callers that
need the byte count, /proc/PID/cmdline embeds NUL, ask for it.

fnread() keeps its signature and becomes a bounded copy out of the
result, so its one caller is unaffected.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

src/cgutil.c

@@ -34,6 +34,7 @@
 #include "cgutil.h"
 #include "initctl.h"
 #include "log.h"
+#include "util.h"
 
 #define CDIM   plain ? "" : "\e[2m"
 #define CRST   plain ? "" : "\e[0m"
@@ -54,42 +55,18 @@ int cgroup_avail(void)
 	return fismnt(FINIT_CGPATH);
 }
 
-static size_t flen(FILE *fp)
-{
-	size_t total = 0, sz;
-	char buf[512];
-
-	while ((sz = fread(buf, 1, sizeof(buf), fp)) > 0)
-		total += sz;
-	rewind(fp);
-
-	return total;
-}
-
 char *pid_cmdline(int pid)
 {
 	size_t i, len;
 	char *buf;
-	FILE *fp;
 
-	fp = fopenf("r", "/proc/%d/cmdline", pid);
-	if (!fp)
+	buf = fslurp(&len, "/proc/%d/cmdline", pid);
+	if (!buf)
 		return strdup("");	/* regular process */
 
-	len = flen(fp);
 	if (len == 0) {
-	fail:
-		fclose(fp);
-		return NULL;		/* kernel thread */
-	}
-
-	buf = calloc(len + 1, 1);
-	if (!buf)
-		goto fail;
-
-	if (fread(buf, 1, len, fp) < len) {
 		free(buf);
-		goto fail;
+		return NULL;		/* kernel thread */
 	}
 
 	/* replace all NUL chars with space */
@@ -98,46 +75,30 @@ char *pid_cmdline(int pid)
 			buf[i] = ' ';
 	}
 
-	fclose(fp);
 	return buf;
 }
 
 char *pid_cgroup(int pid)
 {
-	char *buf, *ptr = NULL;
-	size_t len;
-	FILE *fp;
+	char *buf, *ptr;
 
-	fp = fopenf("r", "/proc/%d/cgroup", pid);
-	if (!fp)
+	buf = fslurp(NULL, "/proc/%d/cgroup", pid);
+	if (!buf)
 		return NULL;
 
-	len = flen(fp);
-	if (len == 0) {
-		fclose(fp);
-		return NULL;
-	}
-	len++;
-
-	buf = calloc(1, len);
-	if (!buf) {
-		fclose(fp);
-		return NULL;
-	}
-
-	if (fgets(buf, len, fp))
-		ptr = chomp(buf);
-	fclose(fp);
+	ptr = strchr(buf, '\n');	/* first line only */
+	if (ptr)
+		*ptr = 0;
+	chomp(buf);
 
+	ptr = strchr(buf, '/');
 	if (ptr) {
-		ptr = strchr(ptr, '/');
-		if (ptr) {
-			memmove(buf, ptr, strlen(ptr) + 1);
-			return buf;
-		}
+		memmove(buf, ptr, strlen(ptr) + 1);
+		return buf;
 	}
 
 	free(buf);
+
 	return NULL;
 }
 

src/conf.c

@@ -1660,30 +1660,13 @@ static int is_new_format(char *file, char *buf)
  */
 static char *conf_read_template(char *file, char *name)
 {
-	struct stat st;
-	char *buf = NULL;
-	size_t len;
-	FILE *fp;
+	char *buf;
 
-	fp = fopen(file, "r");
-	if (!fp)
+	buf = fslurp(NULL, "%s", file);
+	if (!buf)
 		return NULL;
 
-	/* fstat() the open fd, no window for the file to change under us */
-	if (fstat(fileno(fp), &st) || st.st_size < 0)
-		goto out;
-
-	buf = malloc((size_t)st.st_size + 1);
-	if (!buf)
-		goto out;
-
-	len = fread(buf, 1, (size_t)st.st_size, fp);
-	buf[len] = 0;
-	buf = conf_instantiate(buf, name);
-out:
-	fclose(fp);
-
-	return buf;
+	return conf_instantiate(buf, name);
 }
 
 /*

src/util.c

@@ -33,6 +33,7 @@
 #include <regex.h>
 #include <string.h>
 #include <stdarg.h>
+#include <sys/stat.h>
 #include <stdlib.h>
 #ifdef HAVE_TERMIOS_H
 # include <termios.h>
@@ -335,34 +336,109 @@ int mksubsys(const char *dir, mode_t mode, char *user, char *group)
 	return rc;
 }
 
-int fnread(char *buf, size_t len, char *fmt, ...)
+/*
+ * Read an open stream to EOF into a malloc()'ed, NUL terminated buffer.
+ *
+ * st_size is only a hint here, and zero on procfs, so the read loop
+ * runs until EOF rather than trusting it.  Sizing from the already
+ * open fd also leaves no window for the file to change between the
+ * look and the read.
+ */
+static char *slurp(FILE *fp, size_t *lenp)
 {
-	char path[256];
-	va_list ap;
-	FILE *fp;
+	size_t len = 0, size = BUFSIZ;
+	struct stat st;
+	char *buf;
 
-	va_start(ap, fmt);
-	vsnprintf(path, sizeof(path), fmt, ap);
-	va_end(ap);
+	if (!fstat(fileno(fp), &st) && st.st_size > 0 && (size_t)st.st_size > size)
+		size = (size_t)st.st_size;
 
-	if (!buf || !len) {
-		struct stat st;
+	buf = malloc(size + 1);
+	if (!buf)
+		return NULL;
 
-		if (stat(path, &st))
-			return -1;
+	while (1) {
+		char *ptr;
 
-		return (ssize_t)st.st_size;
+		len += fread(&buf[len], 1, size - len, fp);
+		if (len < size)
+			break;		/* EOF, or error caught below */
+
+		ptr = realloc(buf, size * 2 + 1);
+		if (!ptr) {
+			free(buf);
+			return NULL;
+		}
+		buf = ptr;
+		size *= 2;
 	}
 
-	fp = fopen(path, "r");
-	if (!fp)
-		return -1;
+	if (ferror(fp)) {
+		free(buf);
+		return NULL;
+	}
 
-	len = fread(buf, sizeof(char), len - 1, fp);
 	buf[len] = 0;
+	if (lenp)
+		*lenp = len;
+
+	return buf;
+}
+
+/*
+ * Read a whole file into a malloc()'ed, NUL terminated buffer, which
+ * the caller frees.  @lenp, when given, returns the number of bytes
+ * read; the content may itself contain NUL, e.g. /proc/PID/cmdline.
+ */
+char *vfslurp(size_t *lenp, const char *fmt, va_list ap)
+{
+	char *buf;
+	FILE *fp;
+
+	fp = vfopenf("r", fmt, ap);
+	if (!fp)
+		return NULL;
+
+	buf = slurp(fp, lenp);
 	fclose(fp);
 
-	return (int)len;
+	return buf;
+}
+
+char *fslurp(size_t *lenp, const char *fmt, ...)
+{
+	va_list ap;
+	char *buf;
+
+	va_start(ap, fmt);
+	buf = vfslurp(lenp, fmt, ap);
+	va_end(ap);
+
+	return buf;
+}
+
+int fnread(char *buf, size_t len, char *fmt, ...)
+{
+	size_t dlen;
+	va_list ap;
+	char *data;
+
+	va_start(ap, fmt);
+	data = vfslurp(&dlen, fmt, ap);
+	va_end(ap);
+
+	if (!data)
+		return -1;
+
+	if (buf && len) {
+		if (dlen > len - 1)
+			dlen = len - 1;
+		memcpy(buf, data, dlen);
+		buf[dlen] = 0;
+	}
+	free(data);
+
+	return (int)dlen;
 }
 
 int fnwrite(char *value, char *fmt, ...)

src/util.h

@@ -29,6 +29,7 @@
 #include <ctype.h>
 #ifdef HAVE_TERMIOS_H
 #include <poll.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <sys/reboot.h>
 #include <sys/resource.h>
@@ -72,6 +73,8 @@ int   getcgroup    (char *buf, size_t len);
 
 int   mksubsys     (const char *dir, mode_t mode, char *user, char *group);
 
+char *fslurp       (size_t *lenp, const char *fmt, ...)    __attribute__ ((format (printf, 2, 3)));
+char *vfslurp      (size_t *lenp, const char *fmt, va_list ap);
 int   fnread       (char *buf, size_t len, char *fmt, ...) __attribute__ ((format (printf, 3, 4)));
 int   fnwrite      (char *value, char *fmt, ...)           __attribute__ ((format (printf, 2, 3)));
 int   fngetint     (char *path, int *val);