Hi, The cook_args() and raw_args() functions in cat(1) are too clever. They handle multiple special cases in a single big loop with lots of branches. It's been like this since at least 1989:
https://svnweb.freebsd.org/csrg/bin/cat/cat.c?view=markup&pathrev=37179 The goal seems to be to avoid calling cook_buf()/raw_cat() from multiple places. I think the result is convoluted. If we isolated the special cases and called cook_buf()/raw_cat() from multiple places the result would be simpler and flatter. You can break the cleanup in each function into four steps: 1. Pull the no-args case out of the loop and handle it first. Now we don't need to check if (*argv == NULL) in the body of the loop. One fewer assignment to fp/fd, too. 2. In the loop, isolate the (strcmp(*argv, "-") == 0) special case from the normal filename case. Now we don't need to check whether we're working with stdin when we clean up at the end of a loop iteration. Setup and cleanup are adjacent, no additional branches needed. 3. Pass the file name as an argument to cook_buf() and raw_cat(). Now we don't need the global 'filename' variable. Obviously this means we don't need to assign it a value, either. 4. Use a for-loop and move argv iteration into the loop header. Now we increment argv in a single place in the loop. Thoughts? Index: cat.c =================================================================== RCS file: /cvs/src/bin/cat/cat.c,v retrieving revision 1.27 diff -u -p -r1.27 cat.c --- cat.c 28 Jun 2019 13:34:58 -0000 1.27 +++ cat.c 1 Dec 2020 00:24:20 -0000 @@ -51,12 +51,11 @@ extern char *__progname; int bflag, eflag, nflag, sflag, tflag, vflag; int rval; -char *filename; void cook_args(char *argv[]); -void cook_buf(FILE *); +void cook_buf(FILE *, const char *); void raw_args(char *argv[]); -void raw_cat(int); +void raw_cat(int, const char *); int main(int argc, char *argv[]) @@ -110,30 +109,29 @@ cook_args(char **argv) { FILE *fp; - fp = stdin; - filename = "stdin"; - do { - if (*argv) { - if (!strcmp(*argv, "-")) - fp = stdin; - else if ((fp = fopen(*argv, "r")) == NULL) { - warn("%s", *argv); - rval = 1; - ++argv; - continue; - } - filename = *argv++; - } - cook_buf(fp); - if (fp == stdin) + if (*argv == NULL) { + cook_buf(stdin, "stdin"); + return; + } + + for (; *argv != NULL; argv++) { + if (!strcmp(*argv, "-")) { + cook_buf(stdin, "stdin"); clearerr(fp); - else - (void)fclose(fp); - } while (*argv); + continue; + } + if ((fp = fopen(*argv, "r")) == NULL) { + warn("%s", *argv); + rval = 1; + continue; + } + cook_buf(fp, *argv); + (void)fclose(fp); + } } void -cook_buf(FILE *fp) +cook_buf(FILE *fp, const char *filename) { int ch, gobble, line, prev; @@ -200,28 +198,28 @@ raw_args(char **argv) { int fd; - fd = fileno(stdin); - filename = "stdin"; - do { - if (*argv) { - if (!strcmp(*argv, "-")) - fd = fileno(stdin); - else if ((fd = open(*argv, O_RDONLY, 0)) == -1) { - warn("%s", *argv); - rval = 1; - ++argv; - continue; - } - filename = *argv++; + if (*argv == NULL) { + raw_cat(fileno(stdin), "stdin"); + return; + } + + for(; *argv != NULL; argv++) { + if (!strcmp(*argv, "-")) { + raw_cat(fileno(stdin), "stdin"); + continue; + } + if ((fd = open(*argv, O_RDONLY, 0)) == -1) { + warn("%s", *argv); + rval = 1; + continue; } - raw_cat(fd); - if (fd != fileno(stdin)) - (void)close(fd); - } while (*argv); + raw_cat(fd, *argv); + (void)close(fd); + } } void -raw_cat(int rfd) +raw_cat(int rfd, const char *filename) { int wfd; ssize_t nr, nw, off;