sbase

suckless unix tools
git clone git://git.suckless.org/sbase
Log | Files | Refs | README | LICENSE

commit 9a3b12525bd3a2627f5a45c7afe00737a041bb5b
parent 5cb3a1eba1b7568b3c6c32c1e6808de24b38a757
Author: Michael Forney <mforney@mforney.org>
Date:   Sun,  1 Jan 2017 17:00:33 -0800

Don't use buffered IO (fread) when not appropriate

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.

Diffstat:
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 @@ -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" @@ -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; } @@ -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 @@ -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 @@ -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" @@ -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; @@ -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; } @@ -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); } @@ -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); @@ -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); } } @@ -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 @@ -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" @@ -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); @@ -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]; @@ -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 @@ -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; @@ -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 @@ -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" @@ -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; @@ -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; }