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

diff --git a/usr.sbin/cron/atrun.c b/usr.sbin/cron/atrun.c
index 3bd5bde..a00d715 100644 (file)
--- 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 <Todd.Miller@courtesan.com>
@@ -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 17a5632..05971be 100644 (file)
--- 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 363f51b..840c9b9 100644 (file)
--- 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 f359a54..351dd97 100644 (file)
--- 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 0b39959..43e42d7 100644 (file)
--- 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 51a97ee..56c9d1e 100644 (file)
--- 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 7f6b862..f8afdf4 100644 (file)
--- 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 98aca4f..cd5aae9 100644 (file)
--- 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 e730934..96995a8 100644 (file)
--- 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;