diff options
author | Todd C. Miller <millert@cvs.openbsd.org> | 2015-11-09 01:12:28 +0000 |
---|---|---|
committer | Todd C. Miller <millert@cvs.openbsd.org> | 2015-11-09 01:12:28 +0000 |
commit | dde45024e22a1e683b4afb1d55bca6cda0e09030 (patch) | |
tree | 35ed5716966042a685e0dd59a92fc49df778abcd | |
parent | 5b5ef58c30e0cd58c8d01f280d0fe853d50b1f14 (diff) |
queue(3) instead of homegrown queues and lists. This also fixes
some potential memory leaks in error paths. OK guenther@
-rw-r--r-- | usr.sbin/cron/atrun.c | 95 | ||||
-rw-r--r-- | usr.sbin/cron/cron.c | 42 | ||||
-rw-r--r-- | usr.sbin/cron/database.c | 93 | ||||
-rw-r--r-- | usr.sbin/cron/funcs.h | 8 | ||||
-rw-r--r-- | usr.sbin/cron/job.c | 23 | ||||
-rw-r--r-- | usr.sbin/cron/structs.h | 20 | ||||
-rw-r--r-- | usr.sbin/cron/user.c | 19 |
7 files changed, 130 insertions, 170 deletions
diff --git a/usr.sbin/cron/atrun.c b/usr.sbin/cron/atrun.c index c873ed1a164..517be19d6e9 100644 --- a/usr.sbin/cron/atrun.c +++ b/usr.sbin/cron/atrun.c @@ -1,4 +1,4 @@ -/* $OpenBSD: atrun.c,v 1.34 2015/11/04 20:28:17 millert Exp $ */ +/* $OpenBSD: atrun.c,v 1.35 2015/11/09 01:12:27 millert Exp $ */ /* * Copyright (c) 2002-2003 Todd C. Miller <Todd.Miller@courtesan.com> @@ -48,21 +48,20 @@ #include "funcs.h" #include "globals.h" -static void unlink_job(at_db *, atjob *); static void run_job(atjob *, char *); /* * Scan the at jobs dir and build up a list of jobs found. */ int -scan_atjobs(at_db *old_db, struct timespec *ts) +scan_atjobs(at_db **db, struct timespec *ts) { DIR *atdir = NULL; int cwd, queue, pending; time_t run_time; char *ep; - at_db new_db; - atjob *job, *tjob; + at_db *new_db, *old_db = *db; + atjob *job; struct dirent *file; struct stat statbuf; @@ -71,11 +70,11 @@ scan_atjobs(at_db *old_db, struct timespec *ts) return (0); } - if (old_db->mtime == statbuf.st_mtime) { + if (old_db != NULL && old_db->mtime == statbuf.st_mtime) { return (0); } - /* XXX - would be nice to stash the crontab cwd */ + /* XXX - use fstatat/openat instead of chdir */ if ((cwd = open(".", O_RDONLY, 0)) < 0) { log_it("CRON", getpid(), "CAN'T OPEN", "."); return (0); @@ -91,8 +90,14 @@ scan_atjobs(at_db *old_db, struct timespec *ts) return (0); } - new_db.mtime = statbuf.st_mtime; /* stash at dir mtime */ - new_db.head = new_db.tail = NULL; + if ((new_db = malloc(sizeof(*new_db))) == NULL) { + closedir(atdir); + fchdir(cwd); + close(cwd); + return (0); + } + new_db->mtime = statbuf.st_mtime; /* stash at dir mtime */ + TAILQ_INIT(&new_db->jobs); pending = 0; while ((file = readdir(atdir)) != NULL) { @@ -113,11 +118,11 @@ scan_atjobs(at_db *old_db, struct timespec *ts) job = malloc(sizeof(*job)); if (job == NULL) { - for (job = new_db.head; job != NULL; ) { - tjob = job; - job = job->next; - free(tjob); + while ((job = TAILQ_FIRST(&new_db->jobs))) { + TAILQ_REMOVE(&new_db->jobs, job, entries); + free(job); } + free(new_db); closedir(atdir); fchdir(cwd); close(cwd); @@ -127,32 +132,26 @@ scan_atjobs(at_db *old_db, struct timespec *ts) job->gid = statbuf.st_gid; job->queue = queue; job->run_time = run_time; - job->prev = new_db.tail; - job->next = NULL; - if (new_db.head == NULL) - new_db.head = job; - if (new_db.tail != NULL) - new_db.tail->next = job; - new_db.tail = job; + TAILQ_INSERT_TAIL(&new_db->jobs, job, entries); if (ts != NULL && run_time <= ts->tv_sec) pending = 1; } closedir(atdir); - /* Free up old at db */ - for (job = old_db->head; job != NULL; ) { - tjob = job; - job = job->next; - free(tjob); + /* Free up old at db and install new one */ + if (old_db != NULL) { + while ((job = TAILQ_FIRST(&old_db->jobs))) { + TAILQ_REMOVE(&old_db->jobs, job, entries); + free(job); + } + free(old_db); } + *db = new_db; /* Change back to the normal cron dir. */ fchdir(cwd); close(cwd); - /* Install the new database */ - *old_db = new_db; - return (pending); } @@ -165,9 +164,12 @@ atrun(at_db *db, double batch_maxload, time_t now) char atfile[MAX_FNAME]; struct stat statbuf; double la; - atjob *job, *batch; + atjob *job, *tjob, *batch = NULL; + + if (db == NULL) + return; - for (batch = NULL, job = db->head; job; job = job->next) { + TAILQ_FOREACH_SAFE(job, &db->jobs, entries, tjob) { /* Skip jobs in the future */ if (job->run_time > now) continue; @@ -175,11 +177,11 @@ atrun(at_db *db, double batch_maxload, time_t now) snprintf(atfile, sizeof(atfile), "%s/%lld.%c", AT_DIR, (long long)job->run_time, job->queue); - if (stat(atfile, &statbuf) != 0) - unlink_job(db, job); /* disapeared */ - - if (!S_ISREG(statbuf.st_mode)) - continue; /* should not happen */ + if (lstat(atfile, &statbuf) != 0 || !S_ISREG(statbuf.st_mode)) { + TAILQ_REMOVE(&db->jobs, job, entries); + free(job); + continue; /* disapeared or not a file */ + } /* * Pending jobs have the user execute bit set. @@ -194,7 +196,8 @@ atrun(at_db *db, double batch_maxload, time_t now) } else { /* normal at job */ run_job(job, atfile); - unlink_job(db, job); + TAILQ_REMOVE(&db->jobs, job, entries); + free(job); } } } @@ -207,28 +210,12 @@ atrun(at_db *db, double batch_maxload, time_t now) snprintf(atfile, sizeof(atfile), "%s/%lld.%c", AT_DIR, (long long)batch->run_time, batch->queue); run_job(batch, atfile); - unlink_job(db, batch); + TAILQ_REMOVE(&db->jobs, batch, entries); + free(job); } } /* - * Remove the specified at job from the database. - */ -static void -unlink_job(at_db *db, atjob *job) -{ - if (job->prev == NULL) - db->head = job->next; - else - job->prev->next = job->next; - - if (job->next == NULL) - db->tail = job->prev; - else - job->next->prev = job->prev; -} - -/* * Run the specified job contained in atfile. */ static void diff --git a/usr.sbin/cron/cron.c b/usr.sbin/cron/cron.c index 4111850981f..2d5b1e8f271 100644 --- a/usr.sbin/cron/cron.c +++ b/usr.sbin/cron/cron.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cron.c,v 1.63 2015/11/06 23:47:42 millert Exp $ */ +/* $OpenBSD: cron.c,v 1.64 2015/11/09 01:12:27 millert Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC") @@ -59,8 +59,8 @@ static volatile sig_atomic_t got_sighup, got_sigchld; static time_t timeRunning, virtualTime, clockTime; static int cronSock; static long GMToff; -static cron_db database; -static at_db at_database; +static cron_db *database; +static at_db *at_database; static double batch_maxload = BATCH_MAXLOAD; static int NoFork; static time_t StartTime; @@ -118,16 +118,10 @@ main(int argc, char *argv[]) log_it("CRON",getpid(),"STARTUP",CRON_VERSION); } - database.head = NULL; - database.tail = NULL; - database.mtime = 0; load_database(&database); - at_database.head = NULL; - at_database.tail = NULL; - at_database.mtime = 0; scan_atjobs(&at_database, NULL); set_time(TRUE); - run_reboot_jobs(&database); + run_reboot_jobs(database); timeRunning = virtualTime = clockTime; /* @@ -169,7 +163,7 @@ main(int argc, char *argv[]) /* shortcut for the most common case */ if (timeDiff == 1) { virtualTime = timeRunning; - find_jobs(virtualTime, &database, TRUE, TRUE); + find_jobs(virtualTime, database, TRUE, TRUE); } else { if (timeDiff > (3*MINUTE_COUNT) || timeDiff < -(3*MINUTE_COUNT)) @@ -192,7 +186,7 @@ main(int argc, char *argv[]) if (job_runqueue()) sleep(10); virtualTime++; - find_jobs(virtualTime, &database, + find_jobs(virtualTime, database, TRUE, TRUE); } while (virtualTime < timeRunning); break; @@ -210,14 +204,14 @@ main(int argc, char *argv[]) * housekeeping. */ /* run wildcard jobs for current minute */ - find_jobs(timeRunning, &database, TRUE, FALSE); + find_jobs(timeRunning, database, TRUE, FALSE); /* run fixed-time jobs for each minute missed */ do { if (job_runqueue()) sleep(10); virtualTime++; - find_jobs(virtualTime, &database, + find_jobs(virtualTime, database, FALSE, TRUE); set_time(FALSE); } while (virtualTime< timeRunning && @@ -233,7 +227,7 @@ main(int argc, char *argv[]) * not be repeated. Virtual time does not * change until we are caught up. */ - find_jobs(timeRunning, &database, TRUE, FALSE); + find_jobs(timeRunning, database, TRUE, FALSE); break; default: /* @@ -241,7 +235,7 @@ main(int argc, char *argv[]) * jump virtual time, and run everything */ virtualTime = timeRunning; - find_jobs(timeRunning, &database, TRUE, TRUE); + find_jobs(timeRunning, database, TRUE, TRUE); } } @@ -249,7 +243,7 @@ main(int argc, char *argv[]) job_runqueue(); /* Run any jobs in the at queue. */ - atrun(&at_database, batch_maxload, + atrun(at_database, batch_maxload, timeRunning * SECONDS_PER_MINUTE - GMToff); /* Reload jobs as needed. */ @@ -264,8 +258,8 @@ run_reboot_jobs(cron_db *db) user *u; entry *e; - for (u = db->head; u != NULL; u = u->next) { - for (e = u->crontab; e != NULL; e = e->next) { + TAILQ_FOREACH(u, &db->users, entries) { + SLIST_FOREACH(e, &u->crontab, entries) { if (e->flags & WHEN_REBOOT) job_add(e, u); } @@ -296,8 +290,8 @@ find_jobs(time_t vtime, cron_db *db, int doWild, int doNonWild) * is why we keep 'e->dow_star' and 'e->dom_star'. yes, it's bizarre. * like many bizarre things, it's the standard. */ - for (u = db->head; u != NULL; u = u->next) { - for (e = u->crontab; e != NULL; e = e->next) { + TAILQ_FOREACH(u, &db->users, entries) { + SLIST_FOREACH(e, &u->crontab, entries) { if (bit_test(e->minute, minute) && bit_test(e->hour, hour) && bit_test(e->month, month) && @@ -375,7 +369,7 @@ cron_sleep(time_t target, sigset_t *mask) (void) read(fd, &poke, 1); close(fd); if (poke & RELOAD_CRON) { - database.mtime = 0; + database->mtime = 0; load_database(&database); } if (poke & RELOAD_AT) { @@ -385,9 +379,9 @@ cron_sleep(time_t target, sigset_t *mask) * jobs immediately. */ clock_gettime(CLOCK_REALTIME, &t2); - at_database.mtime = 0; + at_database->mtime = 0; if (scan_atjobs(&at_database, &t2)) - atrun(&at_database, + atrun(at_database, batch_maxload, t2.tv_sec); } } diff --git a/usr.sbin/cron/database.c b/usr.sbin/cron/database.c index d9c41d94c08..f0e3990c0f8 100644 --- a/usr.sbin/cron/database.c +++ b/usr.sbin/cron/database.c @@ -1,4 +1,4 @@ -/* $OpenBSD: database.c,v 1.28 2015/11/04 20:28:17 millert Exp $ */ +/* $OpenBSD: database.c,v 1.29 2015/11/09 01:12:27 millert Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC") @@ -43,13 +43,13 @@ static void process_crontab(const char *, const char *, cron_db *, cron_db *); void -load_database(cron_db *old_db) +load_database(cron_db **db) { struct stat statbuf, syscron_stat; - cron_db new_db; + cron_db *new_db, *old_db = *db; struct dirent *dp; DIR *dir; - user *u, *nu; + user *u; /* before we start loading any data, do a stat on SPOOL_DIR * so that if anything changes as of this moment (i.e., before we've @@ -67,12 +67,9 @@ load_database(cron_db *old_db) /* if spooldir's mtime has not changed, we don't need to fiddle with * the database. - * - * Note that old_db->mtime is initialized to 0 in main(), and - * so is guaranteed to be different than the stat() mtime the first - * time this function is called. */ - if (old_db->mtime == HASH(statbuf.st_mtime, syscron_stat.st_mtime)) { + if (old_db != NULL && + old_db->mtime == HASH(statbuf.st_mtime, syscron_stat.st_mtime)) { return; } @@ -81,12 +78,14 @@ load_database(cron_db *old_db) * actually changed. Whatever is left in the old database when * we're done is chaff -- crontabs that disappeared. */ - new_db.mtime = HASH(statbuf.st_mtime, syscron_stat.st_mtime); - new_db.head = new_db.tail = NULL; + if ((new_db = malloc(sizeof(*new_db))) == NULL) + return; + new_db->mtime = HASH(statbuf.st_mtime, syscron_stat.st_mtime); + TAILQ_INIT(&new_db->users); if (syscron_stat.st_mtime) { process_crontab("root", NULL, SYSCRONTAB, &syscron_stat, - &new_db, old_db); + new_db, old_db); } /* we used to keep this dir open all the time, for the sake of @@ -95,6 +94,17 @@ load_database(cron_db *old_db) */ if (!(dir = opendir(SPOOL_DIR))) { log_it("CRON", getpid(), "OPENDIR FAILED", SPOOL_DIR); + /* Restore system crontab entry as needed. */ + if (!TAILQ_EMPTY(&new_db->users) && + (u = TAILQ_FIRST(&old_db->users))) { + if (strcmp(u->name, "*system*") == 0) { + TAILQ_REMOVE(&old_db->users, u, entries); + free_user(u); + TAILQ_INSERT_HEAD(&old_db->users, + TAILQ_FIRST(&new_db->users), entries); + } + } + free(new_db); return; } @@ -117,7 +127,7 @@ load_database(cron_db *old_db) continue; /* XXX log? */ process_crontab(fname, fname, tabname, - &statbuf, &new_db, old_db); + &statbuf, new_db, old_db); } closedir(dir); @@ -129,51 +139,30 @@ load_database(cron_db *old_db) /* whatever's left in the old database is now junk. */ - for (u = old_db->head; u != NULL; u = nu) { - nu = u->next; - unlink_user(old_db, u); - free_user(u); + if (old_db != NULL) { + while ((u = TAILQ_FIRST(&old_db->users))) { + TAILQ_REMOVE(&old_db->users, u, entries); + free_user(u); + } + free(old_db); } /* overwrite the database control block with the new one. */ - *old_db = new_db; -} - -void -link_user(cron_db *db, user *u) -{ - if (db->head == NULL) - db->head = u; - if (db->tail) - db->tail->next = u; - u->prev = db->tail; - u->next = NULL; - db->tail = u; -} - -void -unlink_user(cron_db *db, user *u) -{ - if (u->prev == NULL) - db->head = u->next; - else - u->prev->next = u->next; - - if (u->next == NULL) - db->tail = u->prev; - else - u->next->prev = u->prev; + *db = new_db; } user * find_user(cron_db *db, const char *name) { - user *u; + user *u = NULL; - for (u = db->head; u != NULL; u = u->next) - if (strcmp(u->name, name) == 0) - break; + if (db != NULL) { + TAILQ_FOREACH(u, &db->users, entries) { + if (strcmp(u->name, name) == 0) + break; + } + } return (u); } @@ -234,8 +223,8 @@ process_crontab(const char *uname, const char *fname, const char *tabname, * in, then we can just use our existing entry. */ if (u->mtime == statbuf->st_mtime) { - unlink_user(old_db, u); - link_user(new_db, u); + TAILQ_REMOVE(&old_db->users, u, entries); + TAILQ_INSERT_TAIL(&new_db->users, u, entries); goto next_crontab; } @@ -246,14 +235,14 @@ process_crontab(const char *uname, const char *fname, const char *tabname, * users will be deleted from the old database when * we finish with the crontab... */ - unlink_user(old_db, u); + TAILQ_REMOVE(&old_db->users, u, entries); free_user(u); log_it(fname, getpid(), "RELOAD", tabname); } u = load_user(crontab_fd, pw, fname); if (u != NULL) { u->mtime = statbuf->st_mtime; - link_user(new_db, u); + TAILQ_INSERT_TAIL(&new_db->users, u, entries); } next_crontab: diff --git a/usr.sbin/cron/funcs.h b/usr.sbin/cron/funcs.h index a0229140e45..e15568c3bc5 100644 --- a/usr.sbin/cron/funcs.h +++ b/usr.sbin/cron/funcs.h @@ -1,4 +1,4 @@ -/* $OpenBSD: funcs.h,v 1.24 2015/11/04 12:53:05 millert Exp $ */ +/* $OpenBSD: funcs.h,v 1.25 2015/11/09 01:12:27 millert Exp $ */ /* * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC") @@ -23,11 +23,9 @@ */ void set_cron_cwd(void), - load_database(cron_db *), + load_database(cron_db **), job_add(entry *, user *), do_command(entry *, user *), - link_user(cron_db *, user *), - unlink_user(cron_db *, user *), free_user(user *), env_free(char **), unget_char(int, FILE *), @@ -45,7 +43,7 @@ int job_runqueue(void), cron_pclose(FILE *, pid_t), allowed(const char *, const char *, const char *), safe_p(const char *, const char *), - scan_atjobs(at_db *, struct timespec *); + scan_atjobs(at_db **, struct timespec *); int strtot(const char *nptr, char **endptr, time_t *tp); diff --git a/usr.sbin/cron/job.c b/usr.sbin/cron/job.c index f206ebbf71e..b67b74d5ce1 100644 --- a/usr.sbin/cron/job.c +++ b/usr.sbin/cron/job.c @@ -1,4 +1,4 @@ -/* $OpenBSD: job.c,v 1.12 2015/11/04 20:28:17 millert Exp $ */ +/* $OpenBSD: job.c,v 1.13 2015/11/09 01:12:27 millert Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC") @@ -29,12 +29,13 @@ #include "funcs.h" typedef struct _job { - struct _job *next; + SIMPLEQ_ENTRY(_job) entries; entry *e; user *u; } job; -static job *jhead = NULL, *jtail = NULL; + +static SIMPLEQ_HEAD(job_queue, _job) jobs = SIMPLEQ_HEAD_INITIALIZER(jobs); void job_add(entry *e, user *u) @@ -42,37 +43,31 @@ job_add(entry *e, user *u) job *j; /* if already on queue, keep going */ - for (j = jhead; j != NULL; j = j->next) + SIMPLEQ_FOREACH(j, &jobs, entries) if (j->e == e && j->u == u) return; /* build a job queue element */ if ((j = malloc(sizeof(job))) == NULL) return; - j->next = NULL; j->e = e; j->u = u; /* add it to the tail */ - if (jhead == NULL) - jhead = j; - else - jtail->next = j; - jtail = j; + SIMPLEQ_INSERT_TAIL(&jobs, j, entries); } int job_runqueue(void) { - job *j, *jn; + job *j; int run = 0; - for (j = jhead; j; j = jn) { + while ((j = SIMPLEQ_FIRST(&jobs))) { + SIMPLEQ_REMOVE_HEAD(&jobs, entries); do_command(j->e, j->u); - jn = j->next; free(j); run++; } - jhead = jtail = NULL; return (run); } diff --git a/usr.sbin/cron/structs.h b/usr.sbin/cron/structs.h index d3ac43c88ce..e9e73816b00 100644 --- a/usr.sbin/cron/structs.h +++ b/usr.sbin/cron/structs.h @@ -1,4 +1,4 @@ -/* $OpenBSD: structs.h,v 1.6 2015/11/04 20:28:17 millert Exp $ */ +/* $OpenBSD: structs.h,v 1.7 2015/11/09 01:12:27 millert Exp $ */ /* * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC") @@ -17,10 +17,12 @@ * OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include <sys/queue.h> + struct passwd; typedef struct _entry { - struct _entry *next; + SLIST_ENTRY(_entry) entries; struct passwd *pwd; char **envp; char *cmd; @@ -46,19 +48,19 @@ typedef struct _entry { */ typedef struct _user { - struct _user *next, *prev; /* links */ + TAILQ_ENTRY(_user) entries; /* links */ char *name; time_t mtime; /* last modtime of crontab */ - entry *crontab; /* this person's crontab */ + SLIST_HEAD(crontab_list, _entry) crontab; /* this person's crontab */ } user; typedef struct _cron_db { - user *head, *tail; /* links */ + TAILQ_HEAD(user_list, _user) users; time_t mtime; /* last modtime on spooldir */ } cron_db; typedef struct _atjob { - struct _atjob *next, *prev; /* links */ + TAILQ_ENTRY(_atjob) entries; /* links */ uid_t uid; /* uid of the job */ gid_t gid; /* gid of the job */ int queue; /* name of the at queue */ @@ -66,10 +68,6 @@ typedef struct _atjob { } atjob; typedef struct _at_db { - atjob *head, *tail; /* links */ + TAILQ_HEAD(atjob_list, _atjob) jobs; time_t mtime; /* last modtime on spooldir */ } at_db; - /* in the C tradition, we only create - * variables for the main program, just - * extern them elsewhere. - */ diff --git a/usr.sbin/cron/user.c b/usr.sbin/cron/user.c index d31d58056bc..2b896a32e3e 100644 --- a/usr.sbin/cron/user.c +++ b/usr.sbin/cron/user.c @@ -1,4 +1,4 @@ -/* $OpenBSD: user.c,v 1.16 2015/11/04 20:28:17 millert Exp $ */ +/* $OpenBSD: user.c,v 1.17 2015/11/09 01:12:27 millert Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC") @@ -34,13 +34,13 @@ void free_user(user *u) { - entry *e, *ne; + entry *e; - free(u->name); - for (e = u->crontab; e != NULL; e = ne) { - ne = e->next; + while ((e = SLIST_FIRST(&u->crontab))) { + SLIST_REMOVE_HEAD(&u->crontab, entries); free_entry(e); } + free(u->name); free(u); } @@ -69,7 +69,7 @@ load_user(int crontab_fd, struct passwd *pw, const char *name) errno = save_errno; return (NULL); } - u->crontab = NULL; + SLIST_INIT(&u->crontab); /* init environment. this will be copied/augmented for each entry. */ @@ -86,11 +86,10 @@ load_user(int crontab_fd, struct passwd *pw, const char *name) while ((status = load_env(envstr, file)) >= 0) { switch (status) { case FALSE: + /* Not an env variable, parse as crontab entry. */ e = load_entry(file, NULL, pw, envp); - if (e) { - e->next = u->crontab; - u->crontab = e; - } + if (e) + SLIST_INSERT_HEAD(&u->crontab, e, entries); break; case TRUE: if ((tenvp = env_set(envp, envstr)) == NULL) { |