On Sun, Dec 04, 2016 at 09:55:09PM -0800, Michael Forney wrote: > If we are just copying data from one file to another, we don't need to > fill a complete buffer, just read a chunk at a time, and write it to the > output. > --- > cat.c | 34 ++++++++++------------------------ > libutil/concat.c | 24 ++++++++++++++---------- > libutil/cp.c | 42 ++++++++++++++++-------------------------- > sponge.c | 31 +++++++++++++++++-------------- > text.h | 1 - > util.h | 1 + > xinstall.c | 25 ++++++++++++------------- > 7 files changed, 70 insertions(+), 88 deletions(-) > > diff --git a/cat.c b/cat.c > index e3741aa..55585dd 100644 > --- a/cat.c > +++ b/cat.c > @@ -1,22 +1,11 @@ > /* See LICENSE file for copyright and license details. */ > -#include <stdio.h> > +#include <fcntl.h> > #include <string.h> > #include <unistd.h> > > -#include "text.h" > #include "util.h" > > static void > -uconcat(FILE *fp1, const char *s1, FILE *fp2, const char *s2) > -{ > - int c; > - > - setbuf(fp2, NULL); > - while ((c = getc(fp1)) != EOF) > - putc(c, fp2); > -} > - > -static void > usage(void) > { > eprintf("usage: %s [-u] [file ...]\n", argv0); > @@ -25,37 +14,34 @@ usage(void) > int > main(int argc, char *argv[]) > { > - FILE *fp; > - int ret = 0; > - void (*cat)(FILE *, const char *, FILE *, const char *) = &concat; > + int fd, ret = 0; > > ARGBEGIN { > case 'u': > - cat = &uconcat; > break; > default: > usage(); > } ARGEND > > if (!argc) { > - cat(stdin, "<stdin>", stdout, "<stdout>"); > + if (concat(0, "<stdin>", 1, "<stdout>") < 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; > } > - cat(fp, *argv, stdout, "<stdout>"); > - if (fp != stdin && fshut(fp, *argv)) > + if (concat(fd, *argv, 1, "<stdout>") < 0) > ret = 1; > + if (fd != 0) > + close(fd); > } > } > > - ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>"); > - > return ret; > } > diff --git a/libutil/concat.c b/libutil/concat.c > index fad9471..3f617e2 100644 > --- a/libutil/concat.c > +++ b/libutil/concat.c > @@ -1,19 +1,23 @@ > /* See LICENSE file for copyright and license details. */ > -#include <stdio.h> > +#include <unistd.h> > > -#include "../text.h" > #include "../util.h" > > -void > -concat(FILE *fp1, const char *s1, FILE *fp2, const char *s2) > +int > +concat(int f1, const char *s1, int f2, const char *s2) > { > char buf[BUFSIZ]; > - size_t n; > + ssize_t n; > > - while ((n = fread(buf, 1, sizeof(buf), fp1))) { > - fwrite(buf, 1, n, fp2); > - > - if (feof(fp1) || ferror(fp1) || ferror(fp2)) > - break; > + while ((n = read(f1, buf, sizeof(buf))) > 0) { > + if (writeall(f2, buf, n) < 0) { > + weprintf("write %s:", s2); > + return -1; > + } > + } > + if (n < 0) { > + weprintf("read %s:", s1); > + return -1; > } > + return 0; > } > diff --git a/libutil/cp.c b/libutil/cp.c > index c398962..8cd0a7d 100644 > --- a/libutil/cp.c > +++ b/libutil/cp.c
The stdio.h #include can now also be removed from libutil/cp.c. > @@ -12,7 +12,6 @@ > #include <utime.h> > > #include "../fs.h" > -#include "../text.h" > #include "../util.h" > > int cp_aflag = 0; > @@ -27,7 +26,7 @@ int > cp(const char *s1, const char *s2, int depth) > { > DIR *dp; > - FILE *f1, *f2; > + int f1, f2; > struct dirent *d; > struct stat st; > struct timespec times[2]; > @@ -112,43 +111,34 @@ cp(const char *s1, const char *s2, int depth) > return 0; > } > } else { > - if (!(f1 = fopen(s1, "r"))) { > - weprintf("fopen %s:", s1); > + if ((f1 = open(s1, O_RDONLY)) < 0) { > + weprintf("open %s:", s1); > cp_status = 1; > return 0; > } > - if (!(f2 = fopen(s2, "w"))) { > - if (cp_fflag) { > - if (unlink(s2) < 0 && errno != ENOENT) { > - weprintf("unlink %s:", s2); > - cp_status = 1; > - return 0; > - } else if (!(f2 = fopen(s2, "w"))) { > - weprintf("fopen %s:", s2); > - cp_status = 1; > - return 0; > - } > - } else { > - weprintf("fopen %s:", s2); > + if ((f2 = creat(s2, st.st_mode)) < 0 && cp_fflag) { > + if (unlink(s2) < 0 && errno != ENOENT) { > + weprintf("unlink %s:", s2); > cp_status = 1; > return 0; > } > + f2 = creat(s2, st.st_mode); > } > - concat(f1, s1, f2, s2); > - > - /* preserve permissions by default */ > - fchmod(fileno(f2), st.st_mode); > - > - if (fclose(f2) == EOF) { > - weprintf("fclose %s:", s2); > + if (f2 < 0) { > + weprintf("creat %s:", s2); > cp_status = 1; > return 0; > } > - if (fclose(f1) == EOF) { > - weprintf("fclose %s:", s1); > + if (concat(f1, s1, f2, s2) < 0) { > cp_status = 1; > return 0; > } > + > + /* preserve permissions by default */ > + fchmod(f2, st.st_mode); > + > + close(f1); > + close(f2); > } > > if (cp_aflag || cp_pflag) { > diff --git a/sponge.c b/sponge.c > index baeac7f..da8b28c 100644 > --- a/sponge.c > +++ b/sponge.c > @@ -1,7 +1,8 @@ > /* See LICENSE file for copyright and license details. */ > -#include <stdio.h> > +#include <fcntl.h> > +#include <stdlib.h> > +#include <unistd.h> > > -#include "text.h" > #include "util.h" > > static void > @@ -13,24 +14,26 @@ usage(void) > int > main(int argc, char *argv[]) > { > - FILE *fp, *tmpfp; > - int ret = 0; > + char tmp[] = "/tmp/sponge-XXXXXX"; > + int fd, tmpfd; > > argv0 = argv[0], argc--, argv++; > > if (argc != 1) > usage(); > > - if (!(tmpfp = tmpfile())) > - eprintf("tmpfile:"); > - concat(stdin, "<stdin>", tmpfp, "<tmpfile>"); > - rewind(tmpfp); > + if ((tmpfd = mkstemp(tmp)) < 0) > + eprintf("mkstemp:"); > + unlink(tmp); > + if (concat(0, "<stdin>", tmpfd, "<tmpfile>") < 0) > + return 1; > + if (lseek(tmpfd, 0, SEEK_SET) < 0) > + eprintf("lseek:"); > > - if (!(fp = fopen(argv[0], "w"))) > - eprintf("fopen %s:", argv[0]); > - concat(tmpfp, "<tmpfile>", fp, argv[0]); > + if ((fd = creat(argv[0], 0666)) < 0) Mode 0660 may be the safer option here. Other than that it LGTM! Cheers, Silvan > + eprintf("creat %s:", argv[0]); > + if (concat(tmpfd, "<tmpfile>", fd, argv[0]) < 0) > + return 1; > > - ret |= fshut(fp, argv[0]) | fshut(tmpfp, "<tmpfile>"); > - > - return ret; > + return 0; > } > diff --git a/text.h b/text.h > index bceda52..9858592 100644 > --- a/text.h > +++ b/text.h > @@ -13,5 +13,4 @@ struct linebuf { > #define EMPTY_LINEBUF {NULL, 0, 0,} > void getlines(FILE *, struct linebuf *); > > -void concat(FILE *, const char *, FILE *, const char *); > int linecmp(struct line *, struct line *); > diff --git a/util.h b/util.h > index eaad3ce..968a3af 100644 > --- a/util.h > +++ b/util.h > @@ -64,6 +64,7 @@ int eregcomp(regex_t *, const char *, int); > > /* io */ > ssize_t writeall(int, const void *, size_t); > +int concat(int, const char *, int, const char *); > > /* misc */ > void enmasse(int, char **, int (*)(const char *, const char *, int)); > diff --git a/xinstall.c b/xinstall.c > index 5a0e390..3d24a1a 100644 > --- a/xinstall.c > +++ b/xinstall.c > @@ -2,6 +2,7 @@ > #include <grp.h> > #include <pwd.h> > #include <errno.h> > +#include <fcntl.h> > #include <unistd.h> > #include <stdlib.h> > #include <string.h> > @@ -10,7 +11,6 @@ > #include <sys/wait.h> > > #include "util.h" > -#include "text.h" > > static int Dflag = 0; > static int sflag = 0; > @@ -61,7 +61,7 @@ static int > install(const char *s1, const char *s2, int depth) > { > DIR *dp; > - FILE *f1, *f2; > + int f1, f2; > struct dirent *d; > struct stat st; > ssize_t r; > @@ -109,23 +109,22 @@ install(const char *s1, const char *s2, int depth) > else if (mknod(s2, (st.st_mode & ~07777) | mode, st.st_rdev) < > 0) > eprintf("mknod %s:", s2); > } else { > - if (!(f1 = fopen(s1, "r"))) > - eprintf("fopen %s:", s1); > - if (!(f2 = fopen(s2, "w"))) { > + if ((f1 = open(s1, O_RDONLY)) < 0) > + eprintf("open %s:", s1); > + if ((f2 = creat(s2, 0600)) < 0) { > if (unlink(s2) < 0 && errno != ENOENT) > eprintf("unlink %s:", s2); > - else if (!(f2 = fopen(s2, "w"))) > - eprintf("fopen %s:", s2); > + if ((f2 = creat(s2, 0600)) < 0) > + eprintf("creat %s:", s2); > } > - concat(f1, s1, f2, s2); > + if (concat(f1, s1, f2, s2) < 0) > + exit(1); > > - if (fchmod(fileno(f2), mode) < 0) > + if (fchmod(f2, mode) < 0) > eprintf("fchmod %s:", s2); > > - if (fclose(f2) == EOF) > - eprintf("fclose %s:", s2); > - if (fclose(f1) == EOF) > - eprintf("fclose %s:", s1); > + close(f1); > + close(f2); > > if (sflag) > strip(s2); > -- > 2.11.0 > >