[hackers] [sbase] [PATCH 04/11] Don't use buffered IO (fread) when not appropriate

From: Michael Forney <mforney_AT_mforney.org>
Date: Tue, 6 Dec 2016 02:16:56 -0800

fread reads the entire requested size (BUFSIZ), which causes tools to
block if only small amounts of data are available at a time. At best,
this causes unnecessary copies and inefficiency, at worst, tools like
tee and cat are almost unusable in some cases since they only display
large chunks of data at a time.
---
 cksum.c         | 31 +++++++++++++++++--------------
 crypt.h         |  2 +-
 libutil/crypt.c | 37 +++++++++++++++++++------------------
 od.c            | 43 +++++++++++++++++++++++++++----------------
 tee.c           | 39 +++++++++++++++++++--------------------
 5 files changed, 83 insertions(+), 69 deletions(-)
diff --git a/cksum.c b/cksum.c
index 570ca81..b53ec17 100644
--- a/cksum.c
+++ b/cksum.c
_AT_@ -1,7 +1,9 @@
 /* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
 #include <inttypes.h>
 #include <stdio.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "util.h"
 
_AT_@ -61,19 +63,20 @@ static const unsigned long crctab[] = {         0x00000000,
 };
 
 static void
-cksum(FILE *fp, const char *s)
+cksum(int fd, const char *s)
 {
-	size_t len = 0, i, n;
+	ssize_t n;
+	size_t len = 0, i;
 	uint32_t ck = 0;
 	unsigned char buf[BUFSIZ];
 
-	while ((n = fread(buf, 1, sizeof(buf), fp))) {
+	while ((n = read(fd, buf, sizeof(buf))) > 0) {
 		for (i = 0; i < n; i++)
 			ck = (ck << 8) ^ crctab[(ck >> 24) ^ buf[i]];
 		len += n;
 	}
-	if (ferror(fp)) {
-		weprintf("fread %s:", s ? s : "<stdin>");
+	if (n < 0) {
+		weprintf("read %s:", s ? s : "<stdin>");
 		ret = 1;
 		return;
 	}
_AT_@ -92,29 +95,29 @@ cksum(FILE *fp, const char *s)
 int
 main(int argc, char *argv[])
 {
-	FILE *fp;
+	int fd;
 
 	argv0 = argv[0], argc--, argv++;
 
 	if (!argc) {
-		cksum(stdin, NULL);
+		cksum(0, NULL);
 	} else {
 		for (; *argv; argc--, argv++) {
 			if (!strcmp(*argv, "-")) {
 				*argv = "<stdin>";
-				fp = stdin;
-			} else if (!(fp = fopen(*argv, "r"))) {
-				weprintf("fopen %s:", *argv);
+				fd = 0;
+			} else if ((fd = open(*argv, O_RDONLY)) < 0) {
+				weprintf("open %s:", *argv);
 				ret = 1;
 				continue;
 			}
-			cksum(fp, *argv);
-			if (fp != stdin && fshut(fp, *argv))
-				ret = 1;
+			cksum(fd, *argv);
+			if (fd != 0)
+				close(fd);
 		}
 	}
 
-	ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>");
+	ret |= fshut(stdout, "<stdout>");
 
 	return ret;
 }
diff --git a/crypt.h b/crypt.h
index e0cc08d..2fd2932 100644
--- a/crypt.h
+++ b/crypt.h
_AT_@ -8,5 +8,5 @@ struct crypt_ops {
 
 int cryptcheck(int, char **, struct crypt_ops *, uint8_t *, size_t);
 int cryptmain(int, char **, struct crypt_ops *, uint8_t *, size_t);
-int cryptsum(struct crypt_ops *, FILE *, const char *, uint8_t *);
+int cryptsum(struct crypt_ops *, int, const char *, uint8_t *);
 void mdprint(const uint8_t *, const char *, size_t);
diff --git a/libutil/crypt.c b/libutil/crypt.c
index 6991c39..e285614 100644
--- a/libutil/crypt.c
+++ b/libutil/crypt.c
_AT_@ -1,8 +1,10 @@
 /* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "../crypt.h"
 #include "../text.h"
_AT_@ -41,7 +43,7 @@ static void
 mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
             int *formatsucks, int *noread, int *nonmatch)
 {
-	FILE *fp;
+	int fd;
 	size_t bufsiz = 0;
 	int r;
 	char *line = NULL, *file, *p;
_AT_@ -59,12 +61,12 @@ mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
 		file += 2;
 		for (p = file; *p && *p != '\n' && *p != '\r'; p++); /* strip newline */
 		*p = '\0';
-		if (!(fp = fopen(file, "r"))) {
-			weprintf("fopen %s:", file);
+		if ((fd = open(file, O_RDONLY)) < 0) {
+			weprintf("open %s:", file);
 			(*noread)++;
 			continue;
 		}
-		if (cryptsum(ops, fp, file, md)) {
+		if (cryptsum(ops, fd, file, md)) {
 			(*noread)++;
 			continue;
 		}
_AT_@ -77,7 +79,7 @@ mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
 		} else {
 			(*formatsucks)++;
 		}
-		fclose(fp);
+		close(fd);
 	}
 	free(line);
 }
_AT_@ -124,11 +126,11 @@ cryptcheck(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz
 int
 cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
 {
-	FILE *fp;
+	int fd;
 	int ret = 0;
 
 	if (argc == 0) {
-		if (cryptsum(ops, stdin, "<stdin>", md))
+		if (cryptsum(ops, 0, "<stdin>", md))
 			ret = 1;
 		else
 			mdprint(md, "<stdin>", sz);
_AT_@ -136,18 +138,18 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
 		for (; *argv; argc--, argv++) {
 			if ((*argv)[0] == '-' && !(*argv)[1]) {
 				*argv = "<stdin>";
-				fp = stdin;
-			} else if (!(fp = fopen(*argv, "r"))) {
-				weprintf("fopen %s:", *argv);
+				fd = 0;
+			} else if ((fd = open(*argv, O_RDONLY)) < 0) {
+				weprintf("open %s:", *argv);
 				ret = 1;
 				continue;
 			}
-			if (cryptsum(ops, fp, *argv, md))
+			if (cryptsum(ops, fd, *argv, md))
 				ret = 1;
 			else
 				mdprint(md, *argv, sz);
-			if (fp != stdin && fshut(fp, *argv))
-				ret = 1;
+			if (fd != 0)
+				close(fd);
 		}
 	}
 
_AT_@ -155,16 +157,15 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
 }
 
 int
-cryptsum(struct crypt_ops *ops, FILE *fp, const char *f,
-	 uint8_t *md)
+cryptsum(struct crypt_ops *ops, int fd, const char *f, uint8_t *md)
 {
 	uint8_t buf[BUFSIZ];
-	size_t n;
+	ssize_t n;
 
 	ops->init(ops->s);
-	while ((n = fread(buf, 1, sizeof(buf), fp)) > 0)
+	while ((n = read(fd, buf, sizeof(buf))) > 0)
 		ops->update(ops->s, buf, n);
-	if (ferror(fp)) {
+	if (n < 0) {
 		weprintf("%s: read error:", f);
 		return 1;
 	}
diff --git a/od.c b/od.c
index 27a7104..e5dde83 100644
--- a/od.c
+++ b/od.c
_AT_@ -1,8 +1,10 @@
 /* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "queue.h"
 #include "util.h"
_AT_@ -124,20 +126,23 @@ once:
 	}
 }
 
-static void
-od(FILE *fp, char *fname, int last)
+static int
+od(int fd, char *fname, int last)
 {
 	static unsigned char *line;
 	static size_t lineoff;
 	static off_t addr;
 	unsigned char buf[BUFSIZ];
-	size_t i, n, size = sizeof(buf);
+	size_t i, size = sizeof(buf);
+	ssize_t n;
 
 	while (skip - addr > 0) {
-		n = fread(buf, 1, MIN(skip - addr, sizeof(buf)), fp);
+		n = read(fd, buf, MIN(skip - addr, sizeof(buf)));
+		if (n < 0)
+			weprintf("read %s:", fname);
+		if (n <= 0)
+			return n;
 		addr += n;
-		if (feof(fp) || ferror(fp))
-			return;
 	}
 	if (!line)
 		line = emalloc(linelen);
_AT_@ -145,7 +150,7 @@ od(FILE *fp, char *fname, int last)
 	for (;;) {
 		if (max >= 0)
 			size = MIN(max - (addr - skip), size);
-		if (!(n = fread(buf, 1, size, fp)))
+		if ((n = read(fd, buf, size)) <= 0)
 			break;
 		for (i = 0; i < n; i++, addr++) {
 			line[lineoff++] = buf[i];
_AT_@ -155,10 +160,15 @@ od(FILE *fp, char *fname, int last)
 			}
 		}
 	}
+	if (n < 0) {
+		weprintf("read %s:", fname);
+		return n;
+	}
 	if (lineoff && last)
 		printline(line, lineoff, addr - lineoff);
 	if (last)
 		printline((unsigned char *)"", 0, addr);
+	return 0;
 }
 
 static int
_AT_@ -196,7 +206,7 @@ usage(void)
 int
 main(int argc, char *argv[])
 {
-	FILE *fp;
+	int fd;
 	struct type *t;
 	int ret = 0, len;
 	char *s;
_AT_@ -293,25 +303,26 @@ main(int argc, char *argv[])
 		linelen *= 2;
 
 	if (!argc) {
-		od(stdin, "<stdin>", 1);
+		if (od(0, "<stdin>", 1) < 0)
+			ret = 1;
 	} else {
 		for (; *argv; argc--, argv++) {
 			if (!strcmp(*argv, "-")) {
 				*argv = "<stdin>";
-				fp = stdin;
-			} else if (!(fp = fopen(*argv, "r"))) {
-				weprintf("fopen %s:", *argv);
+				fd = 0;
+			} else if ((fd = open(*argv, O_RDONLY)) < 0) {
+				weprintf("open %s:", *argv);
 				ret = 1;
 				continue;
 			}
-			od(fp, *argv, (!*(argv + 1)));
-			if (fp != stdin && fshut(fp, *argv))
+			if (od(fd, *argv, (!*(argv + 1))) < 0)
 				ret = 1;
+			if (fd != 0)
+				close(fd);
 		}
 	}
 
-	ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>") |
-	       fshut(stderr, "<stderr>");
+	ret |= fshut(stdout, "<stdout>") | fshut(stderr, "<stderr>");
 
 	return ret;
 }
diff --git a/tee.c b/tee.c
index 35e3db5..eac106c 100644
--- a/tee.c
+++ b/tee.c
_AT_@ -1,6 +1,7 @@
 /* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
 #include <signal.h>
-#include <stdio.h>
+#include <unistd.h>
 
 #include "util.h"
 
_AT_@ -13,14 +14,15 @@ usage(void)
 int
 main(int argc, char *argv[])
 {
-	FILE **fps = NULL;
-	size_t i, n, nfps;
-	int ret = 0, aflag = 0, iflag = 0;
+	int *fds = NULL;
+	size_t i, nfds;
+	ssize_t n;
+	int ret = 0, aflag = O_TRUNC, iflag = 0;
 	char buf[BUFSIZ];
 
 	ARGBEGIN {
 	case 'a':
-		aflag = 1;
+		aflag = O_APPEND;
 		break;
 	case 'i':
 		iflag = 1;
_AT_@ -31,31 +33,28 @@ main(int argc, char *argv[])
 
 	if (iflag && signal(SIGINT, SIG_IGN) == SIG_ERR)
 		eprintf("signal:");
-	nfps = argc + 1;
-	fps = ecalloc(nfps, sizeof(*fps));
+	nfds = argc + 1;
+	fds = ecalloc(nfds, sizeof(*fds));
 
 	for (i = 0; i < argc; i++) {
-		if (!(fps[i] = fopen(argv[i], aflag ? "a" : "w"))) {
-			weprintf("fopen %s:", argv[i]);
+		if ((fds[i] = open(argv[i], O_WRONLY|O_CREAT|aflag, 0666)) < 0) {
+			weprintf("open %s:", argv[i]);
 			ret = 1;
 		}
 	}
-	fps[i] = stdout;
+	fds[i] = 1;
 
-	while ((n = fread(buf, 1, sizeof(buf), stdin))) {
-		for (i = 0; i < nfps; i++) {
-			if (fps[i] && fwrite(buf, 1, n, fps[i]) != n) {
-				fshut(fps[i], (i != argc) ? argv[i] : "<stdout>");
-				fps[i] = NULL;
+	while ((n = read(0, buf, sizeof(buf))) > 0) {
+		for (i = 0; i < nfds; i++) {
+			if (fds[i] >= 0 && writeall(fds[i], buf, n) < 0) {
+				weprintf("write %s:", (i != argc) ? argv[i] : "<stdout>");
+				fds[i] = -1;
 				ret = 1;
 			}
 		}
 	}
-
-	ret |= fshut(stdin, "<stdin>");
-	for (i = 0; i < nfps; i++)
-		if (fps[i])
-			ret |= fshut(fps[i], (i != argc) ? argv[i] : "<stdout>");
+	if (n < 0)
+		eprintf("read <stdin>:");
 
 	return ret;
 }
-- 
2.10.2
Received on Tue Dec 06 2016 - 11:16:56 CET

This archive was generated by hypermail 2.3.0 : Tue Dec 06 2016 - 11:24:32 CET