From dd2fc19c1bcbf43e5acf03814c79377818cdc2e1 Mon Sep 17 00:00:00 2001 From: tedu Date: Fri, 23 Jan 2015 02:37:25 +0000 Subject: [PATCH] Remove the OK and ERR macros. They obfuscate the code and don't help legibility. (unix system calls use 0 for ok, but hundreds of other projects use 1 to indicate success.) Despite the name, many system calls (e.g., open) also return not OK values for success. It also cleans up some weird code like int crontab_fd = OK - 1; This diff is mechanical in nature. Later I will fix the bugs it reveals. ok deraadt --- usr.sbin/cron/atrun.c | 6 +++--- usr.sbin/cron/crontab.c | 28 ++++++++++++++-------------- usr.sbin/cron/database.c | 14 +++++++------- usr.sbin/cron/do_command.c | 6 +++--- usr.sbin/cron/entry.c | 4 ++-- usr.sbin/cron/env.c | 6 +++--- usr.sbin/cron/macros.h | 11 +---------- usr.sbin/cron/misc.c | 32 ++++++++++++++++---------------- usr.sbin/cron/user.c | 6 +++--- 9 files changed, 52 insertions(+), 61 deletions(-) diff --git a/usr.sbin/cron/atrun.c b/usr.sbin/cron/atrun.c index 3bd5bdecb1f..a00d715885d 100644 --- a/usr.sbin/cron/atrun.c +++ b/usr.sbin/cron/atrun.c @@ -1,4 +1,4 @@ -/* $OpenBSD: atrun.c,v 1.26 2015/01/23 01:01:06 tedu Exp $ */ +/* $OpenBSD: atrun.c,v 1.27 2015/01/23 02:37:25 tedu Exp $ */ /* * Copyright (c) 2002-2003 Todd C. Miller @@ -225,7 +225,7 @@ run_job(atjob *job, char *atfile) char *nargv[2], *nenvp[1]; /* Open the file and unlink it so we don't try running it again. */ - if ((fd = open(atfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW, 0)) < OK) { + if ((fd = open(atfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW, 0)) < 0) { log_it("CRON", getpid(), "CAN'T OPEN", atfile); return; } @@ -275,7 +275,7 @@ run_job(atjob *job, char *atfile) #endif /* Sanity checks */ - if (fstat(fd, &statbuf) < OK) { + if (fstat(fd, &statbuf) < 0) { log_it(pw->pw_name, getpid(), "FSTAT FAILED", atfile); _exit(EXIT_FAILURE); } diff --git a/usr.sbin/cron/crontab.c b/usr.sbin/cron/crontab.c index 17a5632e0f0..05971bec12c 100644 --- a/usr.sbin/cron/crontab.c +++ b/usr.sbin/cron/crontab.c @@ -1,4 +1,4 @@ -/* $OpenBSD: crontab.c,v 1.68 2015/01/23 01:01:06 tedu Exp $ */ +/* $OpenBSD: crontab.c,v 1.69 2015/01/23 02:37:25 tedu Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * All rights reserved @@ -196,7 +196,7 @@ parse_args(int argc, char *argv[]) { * the race. */ - if (swap_gids() < OK) { + if (swap_gids() < 0) { perror("swapping gids"); exit(EXIT_FAILURE); } @@ -204,7 +204,7 @@ parse_args(int argc, char *argv[]) { perror(Filename); exit(EXIT_FAILURE); } - if (swap_gids_back() < OK) { + if (swap_gids_back() < 0) { perror("swapping gids back"); exit(EXIT_FAILURE); } @@ -312,12 +312,12 @@ edit_cmd(void) { fprintf(stderr, "path too long\n"); goto fatal; } - if (swap_gids() < OK) { + if (swap_gids() < 0) { perror("swapping gids"); exit(EXIT_FAILURE); } t = mkstemp(Filename); - if (swap_gids_back() < OK) { + if (swap_gids_back() < 0) { perror("swapping gids back"); exit(EXIT_FAILURE); } @@ -334,7 +334,7 @@ edit_cmd(void) { copy_crontab(f, NewCrontab); fclose(f); - if (fflush(NewCrontab) < OK) { + if (fflush(NewCrontab) < 0) { perror(Filename); exit(EXIT_FAILURE); } @@ -345,12 +345,12 @@ edit_cmd(void) { fprintf(stderr, "%s: error while writing new crontab to %s\n", ProgramName, Filename); fatal: - if (swap_gids() < OK) { + if (swap_gids() < 0) { perror("swapping gids"); exit(EXIT_FAILURE); } unlink(Filename); - if (swap_gids_back() < OK) { + if (swap_gids_back() < 0) { perror("swapping gids back"); exit(EXIT_FAILURE); } @@ -374,7 +374,7 @@ edit_cmd(void) { goto fatal; } if (timespeccmp(&ts[1], &statbuf.st_mtim, ==)) { - if (swap_gids() < OK) { + if (swap_gids() < 0) { perror("swapping gids"); exit(EXIT_FAILURE); } @@ -383,7 +383,7 @@ edit_cmd(void) { fprintf(stderr, "%s: crontab temp file moved, editor " "may create backup files improperly\n", ProgramName); } - if (swap_gids_back() < OK) { + if (swap_gids_back() < 0) { perror("swapping gids back"); exit(EXIT_FAILURE); } @@ -427,12 +427,12 @@ edit_cmd(void) { goto fatal; } remove: - if (swap_gids() < OK) { + if (swap_gids() < 0) { perror("swapping gids"); exit(EXIT_FAILURE); } unlink(Filename); - if (swap_gids_back() < OK) { + if (swap_gids_back() < 0) { perror("swapping gids back"); exit(EXIT_FAILURE); } @@ -514,7 +514,7 @@ replace_cmd(void) { CheckErrorCount = 0; eof = FALSE; while (!CheckErrorCount && !eof) { switch (load_env(envstr, tmp)) { - case ERR: + case -1: /* check for data before the EOF */ if (envstr[0] != '\0') { Set_LineNum(LineNumber + 1); @@ -539,7 +539,7 @@ replace_cmd(void) { goto done; } - if (fchown(fileno(tmp), pw->pw_uid, -1) < OK) { + if (fchown(fileno(tmp), pw->pw_uid, -1) < 0) { perror("fchown"); fclose(tmp); error = -2; diff --git a/usr.sbin/cron/database.c b/usr.sbin/cron/database.c index 363f51b6241..840c9b9fcbf 100644 --- a/usr.sbin/cron/database.c +++ b/usr.sbin/cron/database.c @@ -1,4 +1,4 @@ -/* $OpenBSD: database.c,v 1.22 2015/01/23 01:01:06 tedu Exp $ */ +/* $OpenBSD: database.c,v 1.23 2015/01/23 02:37:25 tedu Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * All rights reserved @@ -44,14 +44,14 @@ load_database(cron_db *old_db) { * so that if anything changes as of this moment (i.e., before we've * cached any of the database), we'll see the changes next time. */ - if (stat(SPOOL_DIR, &statbuf) < OK) { + if (stat(SPOOL_DIR, &statbuf) < 0) { log_it("CRON", getpid(), "STAT FAILED", SPOOL_DIR); return; } /* track system crontab file */ - if (stat(SYSCRONTAB, &syscron_stat) < OK) + if (stat(SYSCRONTAB, &syscron_stat) < 0) syscron_stat.st_mtime = 0; /* if spooldir's mtime has not changed, we don't need to fiddle with @@ -168,7 +168,7 @@ process_crontab(const char *uname, const char *fname, const char *tabname, struct stat *statbuf, cron_db *new_db, cron_db *old_db) { struct passwd *pw = NULL; - int crontab_fd = OK - 1; + int crontab_fd = -1; user *u; if (fname == NULL) { @@ -182,14 +182,14 @@ process_crontab(const char *uname, const char *fname, const char *tabname, goto next_crontab; } - if ((crontab_fd = open(tabname, O_RDONLY|O_NONBLOCK|O_NOFOLLOW, 0)) < OK) { + if ((crontab_fd = open(tabname, O_RDONLY|O_NONBLOCK|O_NOFOLLOW, 0)) < 0) { /* crontab not accessible? */ log_it(fname, getpid(), "CAN'T OPEN", tabname); goto next_crontab; } - if (fstat(crontab_fd, statbuf) < OK) { + if (fstat(crontab_fd, statbuf) < 0) { log_it(fname, getpid(), "FSTAT FAILED", tabname); goto next_crontab; } @@ -243,7 +243,7 @@ process_crontab(const char *uname, const char *fname, const char *tabname, } next_crontab: - if (crontab_fd >= OK) { + if (crontab_fd >= 0) { close(crontab_fd); } } diff --git a/usr.sbin/cron/do_command.c b/usr.sbin/cron/do_command.c index f359a542142..351dd97939b 100644 --- a/usr.sbin/cron/do_command.c +++ b/usr.sbin/cron/do_command.c @@ -1,4 +1,4 @@ -/* $OpenBSD: do_command.c,v 1.43 2015/01/23 01:03:03 tedu Exp $ */ +/* $OpenBSD: do_command.c,v 1.44 2015/01/23 02:37:25 tedu Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * All rights reserved @@ -448,9 +448,9 @@ child_process(entry *e, user *u) { int waiter; pid_t pid; - while ((pid = wait(&waiter)) < OK && errno == EINTR) + while ((pid = wait(&waiter)) < 0 && errno == EINTR) ; - if (pid < OK) { + if (pid < 0) { break; } /* diff --git a/usr.sbin/cron/entry.c b/usr.sbin/cron/entry.c index 0b399590c02..43e42d789b0 100644 --- a/usr.sbin/cron/entry.c +++ b/usr.sbin/cron/entry.c @@ -1,4 +1,4 @@ -/* $OpenBSD: entry.c,v 1.37 2015/01/23 01:01:06 tedu Exp $ */ +/* $OpenBSD: entry.c,v 1.38 2015/01/23 02:37:25 tedu Exp $ */ /* * Copyright 1988,1990,1993,1994 by Paul Vixie @@ -564,5 +564,5 @@ set_element(bitstr_t *bits, int low, int high, int number) { return (EOF); bit_set(bits, (number-low)); - return (OK); + return (0); } diff --git a/usr.sbin/cron/env.c b/usr.sbin/cron/env.c index 51a97eeb075..56c9d1ec0f3 100644 --- a/usr.sbin/cron/env.c +++ b/usr.sbin/cron/env.c @@ -1,4 +1,4 @@ -/* $OpenBSD: env.c,v 1.26 2015/01/23 01:01:06 tedu Exp $ */ +/* $OpenBSD: env.c,v 1.27 2015/01/23 02:37:25 tedu Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * All rights reserved @@ -121,7 +121,7 @@ enum env_state { ERROR /* Error */ }; -/* return ERR = end of file +/* return -1 = end of file * FALSE = not an env setting (file was repositioned) * TRUE = was an env setting */ @@ -137,7 +137,7 @@ load_env(char *envstr, FILE *f) { fileline = LineNumber; skip_comments(f); if (EOF == get_string(envstr, MAX_ENVSTR, f, "\n")) - return (ERR); + return (-1); bzero(name, sizeof name); bzero(val, sizeof val); diff --git a/usr.sbin/cron/macros.h b/usr.sbin/cron/macros.h index 7f6b8626d47..f8afdf42360 100644 --- a/usr.sbin/cron/macros.h +++ b/usr.sbin/cron/macros.h @@ -1,4 +1,4 @@ -/* $OpenBSD: macros.h,v 1.10 2015/01/23 01:01:06 tedu Exp $ */ +/* $OpenBSD: macros.h,v 1.11 2015/01/23 02:37:25 tedu Exp $ */ /* * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC") @@ -17,17 +17,8 @@ * OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ - /* these are really immutable, and are - * defined for symbolic convenience only - * TRUE, FALSE, and ERR must be distinct - * ERR must be < OK. - */ #define TRUE 1 #define FALSE 0 - /* system calls return this on success */ -#define OK 0 - /* or this on error */ -#define ERR (-1) #define INIT_PID 1 /* parent of orphans */ #define READ_PIPE 0 /* which end of a pipe pair do you read? */ diff --git a/usr.sbin/cron/misc.c b/usr.sbin/cron/misc.c index 98aca4f4a83..cd5aae94c05 100644 --- a/usr.sbin/cron/misc.c +++ b/usr.sbin/cron/misc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.c,v 1.52 2015/01/23 01:01:06 tedu Exp $ */ +/* $OpenBSD: misc.c,v 1.53 2015/01/23 02:37:25 tedu Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * All rights reserved @@ -40,7 +40,7 @@ #define FACILITY LOG_CRON #endif -static int LogFD = ERR; +static int LogFD = -1; #if defined(SYSLOG) static int syslog_open = FALSE; @@ -62,7 +62,7 @@ strcmp_until(const char *left, const char *right, char until) { void set_cron_uid(void) { - if (seteuid(ROOT_UID) < OK) { + if (seteuid(ROOT_UID) < 0) { perror("seteuid"); exit(EXIT_FAILURE); } @@ -78,9 +78,9 @@ set_cron_cwd(void) { #endif /* first check for CRONDIR ("/var/cron" or some such) */ - if (stat(CRONDIR, &sb) < OK && errno == ENOENT) { + if (stat(CRONDIR, &sb) < 0 && errno == ENOENT) { perror(CRONDIR); - if (OK == mkdir(CRONDIR, 0710)) { + if (0 == mkdir(CRONDIR, 0710)) { fprintf(stderr, "%s: created\n", CRONDIR); stat(CRONDIR, &sb); } else { @@ -94,7 +94,7 @@ set_cron_cwd(void) { CRONDIR); exit(EXIT_FAILURE); } - if (chdir(CRONDIR) < OK) { + if (chdir(CRONDIR) < 0) { fprintf(stderr, "cannot chdir(%s), bailing out.\n", CRONDIR); perror(CRONDIR); exit(EXIT_FAILURE); @@ -102,9 +102,9 @@ set_cron_cwd(void) { /* CRONDIR okay (now==CWD), now look at SPOOL_DIR ("tabs" or some such) */ - if (stat(SPOOL_DIR, &sb) < OK && errno == ENOENT) { + if (stat(SPOOL_DIR, &sb) < 0 && errno == ENOENT) { perror(SPOOL_DIR); - if (OK == mkdir(SPOOL_DIR, 0700)) { + if (0 == mkdir(SPOOL_DIR, 0700)) { fprintf(stderr, "%s: created\n", SPOOL_DIR); stat(SPOOL_DIR, &sb); } else { @@ -127,9 +127,9 @@ set_cron_cwd(void) { /* finally, look at AT_DIR ("atjobs" or some such) */ - if (stat(AT_DIR, &sb) < OK && errno == ENOENT) { + if (stat(AT_DIR, &sb) < 0 && errno == ENOENT) { perror(AT_DIR); - if (OK == mkdir(AT_DIR, 0700)) { + if (0 == mkdir(AT_DIR, 0700)) { fprintf(stderr, "%s: created\n", AT_DIR); stat(AT_DIR, &sb); } else { @@ -392,10 +392,10 @@ log_it(const char *username, pid_t xpid, const char *event, const char *detail) if ((msg = malloc(msglen)) == NULL) return; - if (LogFD < OK) { + if (LogFD < 0) { LogFD = open(LOG_FILE, O_WRONLY|O_APPEND|O_CREAT|O_CLOEXEC, 0600); - if (LogFD < OK) { + if (LogFD < 0) { fprintf(stderr, "%s: can't open log file\n", ProgramName); perror(LOG_FILE); @@ -411,8 +411,8 @@ log_it(const char *username, pid_t xpid, const char *event, const char *detail) t->tm_mon+1, t->tm_mday, t->tm_hour, t->tm_min, t->tm_sec, (long)pid, event, detail); - if (LogFD < OK || write(LogFD, msg, strlen(msg)) < OK) { - if (LogFD >= OK) + if (LogFD < 0 || write(LogFD, msg, strlen(msg)) < 0) { + if (LogFD >= 0) perror(LOG_FILE); fprintf(stderr, "%s: can't write to log file\n", ProgramName); write(STDERR_FILENO, msg, strlen(msg)); @@ -439,9 +439,9 @@ log_it(const char *username, pid_t xpid, const char *event, const char *detail) void log_close(void) { - if (LogFD != ERR) { + if (LogFD != -1) { close(LogFD); - LogFD = ERR; + LogFD = -1; } #if defined(SYSLOG) closelog(); diff --git a/usr.sbin/cron/user.c b/usr.sbin/cron/user.c index e73093414dd..96995a89aac 100644 --- a/usr.sbin/cron/user.c +++ b/usr.sbin/cron/user.c @@ -1,4 +1,4 @@ -/* $OpenBSD: user.c,v 1.10 2015/01/23 01:01:06 tedu Exp $ */ +/* $OpenBSD: user.c,v 1.11 2015/01/23 02:37:25 tedu Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * All rights reserved @@ -76,9 +76,9 @@ load_user(int crontab_fd, struct passwd *pw, const char *name) { /* load the crontab */ - while ((status = load_env(envstr, file)) >= OK) { + while ((status = load_env(envstr, file)) >= 0) { switch (status) { - case ERR: + case -1: free_user(u); u = NULL; goto done; -- 2.20.1