Improve the code

Signed-off-by: Mattias Andrée <maandree@kth.se>

1 files changed, 203 insertions, 214 deletions

diff --git a/common.c b/common.c
index 1c16dd2..9ef98f8 100644
--- a/common.c
+++ b/common.c
@@ -4,9 +4,9 @@
 
 #include <sys/stat.h>
 #include <alloca.h>
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -14,11 +14,6 @@
 
 
 
-#define USER_ERROR(string)\
-	(fprintf(stderr, "%s: %s\n", argv0, string), 1)
-
-
-
 /**
  * Storage for binary hash
  */
@@ -30,32 +25,107 @@ static char *restrict hashsum = NULL;
 static char *restrict hexsum = NULL;
 
 /**
- * Storage for binary version of expected checksum
- */
-#define correct_binary  hexsum
-
-/**
- * Whether a mismatch has been found or if a file was missing
- */
-static int bad_found = 0;
-
-/**
  * `argv[0]` from `main`
  */
 char *argv0;
 
 
 
-/**
- * Print usage information and exit
- */
 static void
 usage(void)
 {
 	fprintf(stderr, "usage: %s [-u  | -l | -b | -c] [-R rate] [-C capacity] "
-	                "[(-N | -O) output-size] [(-S | -B) state-size] [-W word-size] "
-	                "[-Z squeeze-count] [-vx] [file ...]", argv0);
-	exit(1);
+	                "[-N output-size] [-S state-size] [-W word-size] "
+	                "[-Z squeeze-count] [-vx] [file ...]\n", argv0);
+	exit(2);
+}
+
+static void
+user_error(const char *text)
+{
+	fprintf(stderr, "%s: %s\n", argv0, text);
+	exit(2);
+}
+
+static void *
+emalloc(size_t n)
+{
+	void *r = malloc(n);
+	if (!r) {
+		perror(argv0);
+		exit(2);
+	}
+	return r;
+}
+
+static void *
+erealloc(void *ptr, size_t n)
+{
+	if (!(ptr = realloc(ptr, n))) {
+		perror(argv0);
+		exit(2);
+	}
+	return ptr;
+}
+
+static void
+eperror(void)
+{
+	perror(argv0);
+	exit(2);
+}
+
+
+/**
+ * Convert `libkeccak_generalised_spec_t` to `libkeccak_spec_t` and check for errors
+ * 
+ * @param  gspec  See libkeccak_degeneralise_spec(3)
+ * @param  spec   See libkeccak_degeneralise_spec(3)
+ */
+static void
+make_spec(libkeccak_generalised_spec_t *restrict gspec, libkeccak_spec_t *restrict spec)
+{
+#define case /* fall through */ case
+#define default /* fall through */ default
+#define TEST(CASE, STR) case LIBKECCAK_GENERALISED_SPEC_ERROR_##CASE: user_error(STR)
+	switch (libkeccak_degeneralise_spec(gspec, spec)) {
+	case 0:
+		break;
+	TEST (STATE_NONPOSITIVE,      "the state size must be positive");
+	TEST (STATE_TOO_LARGE,        "the state size is too large, may not exceed 1600");
+	TEST (STATE_MOD_25,           "the state size must be a multiple of 25");
+	TEST (WORD_NONPOSITIVE,       "the word size must be positive");
+	TEST (WORD_TOO_LARGE,         "the word size is too large, may not exceed 64");
+	TEST (STATE_WORD_INCOHERENCY, "the state size must be exactly 25 times the word size");
+	TEST (CAPACITY_NONPOSITIVE,   "the capacity must be positive");
+	TEST (CAPACITY_MOD_8,         "the capacity must be a multiple of 8");
+	TEST (BITRATE_NONPOSITIVE,    "the rate must be positive");
+	TEST (BITRATE_MOD_8,          "the rate must be a multiple of 8");
+	TEST (OUTPUT_NONPOSITIVE,     "the output size must be positive");
+	default:
+		user_error("unknown error in algorithm parameters");
+	}
+#undef TEST
+
+#define TEST(CASE, STR) case LIBKECCAK_SPEC_ERROR_##CASE: user_error(STR)
+	switch (libkeccak_spec_check(spec)) {
+	case 0:
+		break;
+	TEST (BITRATE_NONPOSITIVE,  "the rate size must be positive");
+	TEST (BITRATE_MOD_8,        "the rate must be a multiple of 8");
+	TEST (CAPACITY_NONPOSITIVE, "the capacity must be positive");
+	TEST (CAPACITY_MOD_8,       "the capacity must be a multiple of 8");
+	TEST (OUTPUT_NONPOSITIVE,   "the output size must be positive");
+	TEST (STATE_TOO_LARGE,      "the state size is too large, may not exceed 1600");
+	TEST (STATE_MOD_25,         "the state size must be a multiple of 25");
+	TEST (WORD_NON_2_POTENT,    "the word size must be a power of 2");
+	TEST (WORD_MOD_8,           "the word size must be a multiple of 8");
+	default:
+		user_error("unknown error in algorithm parameters");
+	}
+#undef TEST
+#undef default
+#undef case
 }
 
 
@@ -78,31 +148,33 @@ generalised_sum_fd_hex(int fd, libkeccak_state_t *restrict state,
 {
 	ssize_t got;
 	struct stat attr;
-	size_t blksize = 4096, r_ptr = 0, w_ptr = 0;
+	size_t blksize = 4096, r = 0, w = 0;
 	char *restrict chunk;
 	char even = 1, buf = 0, c;
 
 	if (libkeccak_state_initialise(state, spec) < 0)
 		return -1;
 
-	if (fstat(fd, &attr) == 0)
-		if (attr.st_blksize > 0)
-			blksize = (size_t)(attr.st_blksize);
+	if (!fstat(fd, &attr) && attr.st_blksize > 0)
+		blksize = (size_t)(attr.st_blksize);
 
 	chunk = alloca(blksize);
 
 	for (;;) {
 		got = read(fd, chunk, blksize);
-		if (got < 0) return -1;
-		if (!got)    break;
-		while (r_ptr < (size_t)got) {
-			if (c = chunk[r_ptr++], c <= ' ')
+		if (got < 0)
+			return -1;
+		if (!got)
+			break;
+		while (r < (size_t)got) {
+			c = chunk[r++];
+			if (c <= ' ')
 				continue;
 			buf = (buf << 4) | ((c & 15) + (c > '9' ? 9 : 0));
 			if ((even ^= 1))
-				chunk[w_ptr++] = buf;
+				chunk[w++] = buf;
 		}
-		if (libkeccak_fast_update(state, chunk, w_ptr) < 0)
+		if (libkeccak_fast_update(state, chunk, w) < 0)
 			return -1;
 	}
 
@@ -111,95 +183,46 @@ generalised_sum_fd_hex(int fd, libkeccak_state_t *restrict state,
 
 
 /**
- * Convert `libkeccak_generalised_spec_t` to `libkeccak_spec_t` and check for errors
- * 
- * @param   gspec  See `libkeccak_degeneralise_spec` 
- * @param   spec   See `libkeccak_degeneralise_spec` 
- * @return         Zero on success, an appropriate exit value on error
- */
-static int
-make_spec(libkeccak_generalised_spec_t *restrict gspec, libkeccak_spec_t *restrict spec)
-{
-	int r;
-
-#define TEST(CASE, STR) case LIBKECCAK_GENERALISED_SPEC_ERROR_##CASE: return USER_ERROR(STR)
-	if (r = libkeccak_degeneralise_spec(gspec, spec), r) {
-		switch (r) {
-		TEST (STATE_NONPOSITIVE,      "the state size must be positive");
-		TEST (STATE_TOO_LARGE,        "the state size is too large, may not exceed 1600");
-		TEST (STATE_MOD_25,           "the state size must be a multiple of 25");
-		TEST (WORD_NONPOSITIVE,       "the word size must be positive");
-		TEST (WORD_TOO_LARGE,         "the word size is too large, may not exceed 64");
-		TEST (STATE_WORD_INCOHERENCY, "the state size must be exactly 25 times the word size");
-		TEST (CAPACITY_NONPOSITIVE,   "the capacity must be positive");
-		TEST (CAPACITY_MOD_8,         "the capacity must be a multiple of 8");
-		TEST (BITRATE_NONPOSITIVE,    "the rate must be positive");
-		TEST (BITRATE_MOD_8,          "the rate must be a multiple of 8");
-		TEST (OUTPUT_NONPOSITIVE,     "the output size must be positive");
-		default:
-			return USER_ERROR("unknown error in algorithm parameters");
-		}
-	}
-#undef TEST
-
-#define TEST(CASE, STR) case LIBKECCAK_SPEC_ERROR_##CASE: return USER_ERROR(STR)
-	if (r = libkeccak_spec_check(spec), r) {
-		switch (r) {
-		TEST (BITRATE_NONPOSITIVE,  "the rate size must be positive");
-		TEST (BITRATE_MOD_8,        "the rate must be a multiple of 8");
-		TEST (CAPACITY_NONPOSITIVE, "the capacity must be positive");
-		TEST (CAPACITY_MOD_8,       "the capacity must be a multiple of 8");
-		TEST (OUTPUT_NONPOSITIVE,   "the output size must be positive");
-		TEST (STATE_TOO_LARGE,      "the state size is too large, may not exceed 1600");
-		TEST (STATE_MOD_25,         "the state size must be a multiple of 25");
-		TEST (WORD_NON_2_POTENT,    "the word size must be a power of 2");
-		TEST (WORD_MOD_8,           "the word size must be a multiple of 8");
-		default:
-			return USER_ERROR("unknown error in algorithm parameters");
-		}
-	}
-#undef TEST
-
-	return 0;
-}
-
-
-/**
  * Calculate the checksum of a file and store it in the global variable `hashsum`
  * 
- * @param   filename        The file to hash
- * @param   spec            Hashing parameters
- * @param   squeezes        The number of squeezes to perform
- * @param   suffix          The message suffix
- * @param   hex             Whether to use hexadecimal input rather than binary
- * @return                  Zero on success, an appropriate exit value on error
+ * @param   filename  The file to hash
+ * @param   spec      Hashing parameters
+ * @param   squeezes  The number of squeezes to perform
+ * @param   suffix    The message suffix
+ * @param   hex       Whether to use hexadecimal input rather than binary
+ * @return            An appropriate exit value
  */
 static int
 hash(const char *restrict filename, const libkeccak_spec_t *restrict spec,
      long squeezes, const char *restrict suffix, int hex)
 {
+	static size_t length = 0;
 	libkeccak_state_t state;
-	size_t length;
-	int r, fd;
-
-	length = (size_t)((spec->output + 7) / 8);
-
-	if (!hashsum && (hashsum = malloc(length * sizeof(char)), !hashsum))
-		return perror(argv0), 2;
+	int fd;
 
-	if (!hexsum && (hexsum = malloc((length * 2 + 1) * sizeof(char)), !hexsum))
-		return perror(argv0), 2;
+	if (!length) {
+		length = (size_t)((spec->output + 7) / 8);
+		hashsum = emalloc(length * sizeof(char));
+ 		hexsum = emalloc((length * 2 + 1) * sizeof(char));
+	}
 
-	if (fd = open(strcmp(filename, "-") ? filename : "/dev/stdin", O_RDONLY), fd < 0)
-		return r = (errno != ENOENT), perror(argv0), r + 1;
+	filename = strcmp(filename, "-") ? filename : "/dev/stdin";
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		if (errno == ENOENT)
+			return 1;
+		eperror();
+	}
 
-	if ((hex == 0 ? libkeccak_generalised_sum_fd : generalised_sum_fd_hex)
+	if ((hex ? generalised_sum_fd_hex : libkeccak_generalised_sum_fd)
 	    (fd, &state, spec, suffix, squeezes > 1 ? NULL : hashsum))
-		return perror(argv0), close(fd), libkeccak_state_fast_destroy(&state), 2;
+		eperror();
 	close(fd);
 
-	if (squeezes > 2) libkeccak_fast_squeeze(&state, squeezes - 2);
-	if (squeezes > 1) libkeccak_squeeze(&state, hashsum);
+	if (squeezes > 2)
+		libkeccak_fast_squeeze(&state, squeezes - 2);
+	if (squeezes > 1)
+		libkeccak_squeeze(&state, hashsum);
 	libkeccak_state_fast_destroy(&state);
 
 	return 0;
@@ -216,30 +239,27 @@ hash(const char *restrict filename, const libkeccak_spec_t *restrict spec,
  * @param   hex           Whether to use hexadecimal input rather than binary
  * @param   filename      The file to check
  * @param   correct_hash  The expected checksum (any form of hexadecimal)
- * @return                Zero on success, an appropriate exit value on error
+ * @return                An appropriate exit value
  */
 static int
 check(const libkeccak_spec_t *restrict spec, long squeezes, const char *restrict suffix,
       int hex, const char *restrict filename, const char *restrict correct_hash)
 {
 	size_t length = (size_t)((spec->output + 7) / 8);
-	int r;
 
-	if (access(filename, F_OK)) {
-		bad_found = 1;
-		printf("%s: %s\n", filename, "Missing");
-		return 0;
+	if (access(filename, F_OK) || hash(filename, spec, squeezes, suffix, hex)) {
+		printf("%s: Missing\n", filename);
+		return 1;
 	}
 
-	if ((r = hash(filename, spec, squeezes, suffix, hex)))
-		return r;
-
-	libkeccak_unhex(correct_binary, correct_hash);
-	if ((r = memcmp(correct_binary, hashsum, length)))
-		bad_found = 1;
-	printf("%s: %s\n", filename, !r ? "OK" : "Fail");
-
-	return 0;
+	libkeccak_unhex(hexsum, correct_hash);
+	if (memcmp(hexsum, hashsum, length)) {
+		printf("%s: Fail\n", filename);
+		return 1;
+	} else {
+		printf("%s: OK\n", filename);
+		return 0;
+	}
 }
 
 
@@ -252,7 +272,7 @@ check(const libkeccak_spec_t *restrict spec, long squeezes, const char *restrict
  * @param   suffix    The message suffix
  * @param   style     (unused)
  * @param   hex       Whether to use hexadecimal input rather than binary
- * @return            Zero on success, an appropriate exit value on error
+ * @return            An appropriate exit value
  */
 static int
 check_checksums(const char *restrict filename, const libkeccak_spec_t *restrict spec,
@@ -263,9 +283,10 @@ check_checksums(const char *restrict filename, const libkeccak_spec_t *restrict
 	size_t size = 4096;
 	size_t ptr = 0;
 	ssize_t got;
-	char *buf = NULL;
-	char *new;
-	int fd = -1, rc = 2, stage, r;
+	char *buf;
+	int fd = -1;
+	int ret = 0;
+	int stage;
 	size_t hash_start = 0, hash_end = 0;
 	size_t file_start = 0, file_end = 0;
 	char *hash;
@@ -273,35 +294,33 @@ check_checksums(const char *restrict filename, const libkeccak_spec_t *restrict
 	size_t hash_n;
 	char c;
 
-	if (fd = open(strcmp(filename, "-") ? filename : "/dev/stdin", O_RDONLY), fd < 0)
-		goto pfail;
+	fd = open(strcmp(filename, "-") ? filename : "/dev/stdin", O_RDONLY);
+	if (fd < 0)
+		eperror();
 
-	if (fstat(fd, &attr) == 0) {
-		if (attr.st_blksize > 0) blksize = (size_t)(attr.st_blksize);
-		if (attr.st_size    > 0) size    = (size_t)(attr.st_size);
+	if (!fstat(fd, &attr)) {
+		if (attr.st_blksize > 0)
+			blksize = (size_t)(attr.st_blksize);
+		if (attr.st_size > 0)
+			size = (size_t)(attr.st_size);
 	}
 
 	size = size > blksize ? size : blksize;
-	if (buf = malloc(size), buf == NULL)
-		goto pfail;
+	buf = emalloc(size);
 
 	for (;;) {
-		if (ptr + blksize < size) {
-			if (new = realloc(buf, size <<= 1), new == NULL)
-				goto pfail;
-			buf = new;
-		}
+		if (ptr + blksize < size)
+			buf = erealloc(buf, size <<= 1);
 
 		got = read(fd, buf + ptr, blksize);
-		if      (got < 0)  goto pfail;
-		else if (got == 0) break;
-		else               ptr += (size_t)got;
-	}
-	if (ptr == size) {
-		if (new = realloc(buf, size + 1), new == NULL)
-			goto pfail;
-		buf = new;
+		if (got < 0)
+			eperror();
+		if (!got)
+			break;
+		ptr += (size_t)got;
 	}
+	if (ptr == size)
+		buf = erealloc(buf, size + 1);
 	size = ptr;
 	close(fd), fd = -1;
 	buf[size++] = '\n';
@@ -309,71 +328,50 @@ check_checksums(const char *restrict filename, const libkeccak_spec_t *restrict
 	for (ptr = 0, stage = 0; ptr < size; ptr++) {
 		c = buf[ptr];
 		if (stage == 0) {
-			if      (('0' <= c) && (c <= '9'));
-			else if (('a' <= c) && (c <= 'f'));
-			else if (('A' <= c) && (c <= 'F'));
-			else if ((c == ' ') || (c == '\t')) {
+			if (isxdigit(c))
+				;
+			else if (c == ' ' || c == '\t')
 				hash_end = ptr, stage++;
-			} else if ((c == '\n') || (c == '\f') || (c == '\r')) {
+			else if (c == '\n' || c == '\f' || c == '\r')
 				hash_end = ptr, stage = 3;
-			} else {
-				rc = USER_ERROR("file is malformated");
-				goto fail;
-			}
+			else
+				user_error("file is malformated");
 		} else if (stage == 1) {
-			if ((c == '\n') || (c == '\f') || (c == '\r'))
+			if (c == '\n' || c == '\f' || c == '\r')
 				stage = 3;
-			else if ((c != ' ') && (c != '\t'))
+			else if (c != ' ' && c != '\t')
 				file_start = ptr, stage++;
 		} else if (stage == 2) {
-			if ((c == '\n') || (c == '\f') || (c == '\r'))
+			if (c == '\n' || c == '\f' || c == '\r')
 				file_end = ptr, stage++;
 		}
 
 		if (stage == 3) {
-			if ((hash_start == hash_end) != (file_start == file_end)) {
-				rc = USER_ERROR("file is malformated");
-				goto fail;
-			}
+			if ((hash_start == hash_end) != (file_start == file_end))
+				user_error("file is malformated");
 			if (hash_start != hash_end) {
 				hash = buf + hash_start;
 				file = buf + file_start;
 				hash_n = hash_end - hash_start;
 				buf[hash_end] = '\0';
 				buf[file_end] = '\0';
-				if (hash_n % 2) {
-					rc = USER_ERROR("file is malformated");
-					goto fail;
-				}
-				if (hash_n / 2 != (size_t)((spec->output + 7) / 8)) {
-					rc = USER_ERROR("algorithm parameter mismatch");
-					goto fail;
-				}
-				if ((r = check(spec, squeezes, suffix, hex, file, hash))) {
-					rc = r;
-					goto fail;
-				}
+				if (hash_n % 2)
+					user_error("file is malformated");
+				if (hash_n / 2 != (size_t)((spec->output + 7) / 8))
+					user_error("algorithm parameter mismatch");
+				ret |= check(spec, squeezes, suffix, hex, file, hash);
 			}
 			stage = 0;
 			hash_start = hash_end = file_start = file_end = ptr + 1;
 		}
 	}
 
-	if (stage) {
-		rc = USER_ERROR("file is malformated");
-		goto fail;
-	}
+	if (stage)
+		user_error("file is malformated");
 
 	free(buf);
-	return 0;
-
-pfail:
-	perror(argv0);
-fail:
-	free(buf);
-	if (fd >= 0)
-		close(fd);
-	return rc;
+	close(fd);
+	return ret;
 
 	(void) style;
 }
@@ -388,34 +386,31 @@ fail:
  * @param   suffix    The message suffix
  * @param   style     How the hashes shall be represented
  * @param   hex       Whether to use hexadecimal input rather than binary
- * @return            Zero on success, an appropriate exit value on error
+ * @return            An appropriate exit value
  */
 static int
 print_checksum(const char *restrict filename, const libkeccak_spec_t *restrict spec,
                long squeezes, const char *restrict suffix, enum representation style, int hex)
 {
-	size_t length = (size_t)((spec->output + 7) / 8);
-	int r;
-	size_t ptr = 0;
-	ssize_t wrote;
+	size_t p = 0, n = (size_t)((spec->output + 7) / 8);
+	ssize_t w;
 
-	if ((r = hash(filename, spec, squeezes, suffix, hex)))
-		return r;
+	if (hash(filename, spec, squeezes, suffix, hex)) {
+		fprintf(stderr, "%s: %s: %s\n", argv0, filename, strerror(errno));
+		return 1;
+	}
 
 	if (style == REPRESENTATION_UPPER_CASE) {
-		libkeccak_behex_upper(hexsum, hashsum, length);
+		libkeccak_behex_upper(hexsum, hashsum, n);
 		printf("%s  %s\n", hexsum, filename);
 	} else if (style == REPRESENTATION_LOWER_CASE) {
-		libkeccak_behex_lower(hexsum, hashsum, length);
+		libkeccak_behex_lower(hexsum, hashsum, n);
 		printf("%s  %s\n", hexsum, filename);
 	} else {
 		fflush(stdout);
-		while (length - ptr) {
-			wrote = write(STDOUT_FILENO, hashsum, length - ptr);
-			if (wrote <= 0)
-				return perror(argv0), 2;
-			ptr += (size_t)wrote;
-		}
+		for (; p < n; p += (size_t)w)
+			if ((w = write(STDOUT_FILENO, &hashsum[p], n - p)) < 0)
+				eperror();
 	}
 
 	return 0;
@@ -442,7 +437,7 @@ run(int argc, char *argv[], libkeccak_generalised_spec_t *restrict gspec, const
 	int (*fun)(const char *restrict filename, const libkeccak_spec_t *restrict spec,
 	           long squeezes, const char *restrict suffix, enum representation style, int hex);
 	libkeccak_spec_t spec;
-	int r;
+	int r = 0;
 
 	ARGBEGIN {
 	case 'R':
@@ -492,13 +487,9 @@ run(int argc, char *argv[], libkeccak_generalised_spec_t *restrict gspec, const
 
 	fun = check ? check_checksums : print_checksum;
 
-	if ((r = make_spec(gspec, &spec)))
-		goto done;
-
-	if (squeezes <= 0) {
-		r = USER_ERROR("the squeeze count most be positive");
-		goto done;
-	}
+	make_spec(gspec, &spec);
+	if (squeezes <= 0)
+		user_error("the squeeze count most be positive");
 
 	if (verbose) {
 		fprintf(stderr,        "rate: %li\n", gspec->bitrate);
@@ -513,11 +504,9 @@ run(int argc, char *argv[], libkeccak_generalised_spec_t *restrict gspec, const
 	if (!*argv)
 		r = fun("-", &spec, squeezes, suffix, style, hex);
 	for (; *argv; argv++)
-		if ((r = fun(*argv, &spec, squeezes, suffix, style, hex)))
-			break;
+		r |= fun(*argv, &spec, squeezes, suffix, style, hex);
 
-done:
 	free(hashsum);
 	free(hexsum);
-	return r ? r : bad_found;
+	return r;
 }