From 695153ac183e3e2aaea0961339007f34bf98d47c Mon Sep 17 00:00:00 2001 From: FRIGN Date: Wed, 11 Mar 2015 11:16:40 +0100 Subject: [PATCH] Audit cmp(1) 1) Remove the return-value-enum, which is not necessary for a simple program like this. 2) Don't disallow both l and s to be specified. This is undefined behaviour defined by POSIX, so we don't start demanding things from the user. 3) Replace exit() with return (we are in main). 4) Refactor main loop to never return in the loop, but actually set the same-value and break, which increases readability. 5) Remove the final fclose()'s. The OS will take care of them, no need to become cleansy here. 6) Use idiomatic return-value using same. This concludes the increase of readability in the main-loop. --- README | 2 +- cmp.c | 34 ++++++++++++++-------------------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/README b/README index f5a17bb..132287b 100644 --- a/README +++ b/README @@ -17,7 +17,7 @@ The following tools are implemented ('*' == finished, '#' == UTF-8 support, =*| chown yes none =*| chroot non-posix none =*| cksum yes none -=* cmp yes none +=*| cmp yes none #*| cols non-posix none col yes none =* comm yes none diff --git a/cmp.c b/cmp.c index 9d9f41e..1e48c52 100644 --- a/cmp.c +++ b/cmp.c @@ -4,12 +4,10 @@ #include "util.h" -enum { Same = 0, Diff = 1, Error = 2 }; - static void usage(void) { - enprintf(Error, "usage: %s [-l | -s] file1 file2\n", argv0); + enprintf(2, "usage: %s [-l | -s] file1 file2\n", argv0); } int @@ -30,7 +28,7 @@ main(int argc, char *argv[]) usage(); } ARGEND; - if (argc != 2 || (lflag && sflag)) + if (argc != 2) usage(); for (n = 0; n < 2; n++) { @@ -38,11 +36,10 @@ main(int argc, char *argv[]) argv[n] = ""; fp[n] = stdin; } else { - fp[n] = fopen(argv[n], "r"); - if (!fp[n]) { + if (!(fp[n] = fopen(argv[n], "r"))) { if (!sflag) weprintf("fopen %s:", argv[n]); - exit(Error); + return 2; } } } @@ -57,25 +54,22 @@ main(int argc, char *argv[]) else if (b[0] == '\n') line++; continue; - } - if (b[0] == EOF || b[1] == EOF) { + } else if (b[0] == EOF || b[1] == EOF) { if (!sflag) - fprintf(stderr, "cmp: EOF on %s\n", - argv[(b[0] == EOF) ? 0 : 1]); - exit(Diff); - } - if (!lflag) { + weprintf("cmp: EOF on %s\n", argv[(b[0] != EOF)]); + same = 0; + break; + } else if (!lflag) { if (!sflag) - printf("%s %s differ: byte %ld, line %ld\n", + printf("%s %s differ: byte %zu, line %zu\n", argv[0], argv[1], n, line); - exit(Diff); + same = 0; + break; } else { - printf("%ld %o %o\n", n, b[0], b[1]); + printf("%zu %o %o\n", n, b[0], b[1]); same = 0; } } - fclose(fp[0]); - fclose(fp[1]); - return same ? Same : Diff; + return !same; }