Use queue(3) instead of homegrown queues and lists. This also fixes
some potential memory leaks in error paths.
- todd
Index: usr.sbin/cron/atrun.c
===================================================================
RCS file: /cvs/src/usr.sbin/cron/atrun.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 atrun.c
--- usr.sbin/cron/atrun.c 4 Nov 2015 20:28:17 -0000 1.34
+++ usr.sbin/cron/atrun.c 8 Nov 2015 14:40:54 -0000
@@ -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 timesp
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 timesp
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 timesp
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 timesp
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, t
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, t
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, t
} else {
/* normal at job */
run_job(job, atfile);
- unlink_job(db, job);
+ TAILQ_REMOVE(&db->jobs, job, entries);
+ free(job);
}
}
}
@@ -207,25 +210,9 @@ atrun(at_db *db, double batch_maxload, t
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;
}
/*
Index: usr.sbin/cron/cron.c
===================================================================
RCS file: /cvs/src/usr.sbin/cron/cron.c,v
retrieving revision 1.63
diff -u -p -u -r1.63 cron.c
--- usr.sbin/cron/cron.c 6 Nov 2015 23:47:42 -0000 1.63
+++ usr.sbin/cron/cron.c 8 Nov 2015 14:40:54 -0000
@@ -59,8 +59,8 @@ static volatile sig_atomic_t got_sighup,
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
* 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);
}
}
Index: usr.sbin/cron/database.c
===================================================================
RCS file: /cvs/src/usr.sbin/cron/database.c,v
retrieving revision 1.28
diff -u -p -u -r1.28 database.c
--- usr.sbin/cron/database.c 4 Nov 2015 20:28:17 -0000 1.28
+++ usr.sbin/cron/database.c 8 Nov 2015 14:40:54 -0000
@@ -43,13 +43,13 @@ static void process_crontab(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
* 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
* 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:
Index: usr.sbin/cron/funcs.h
===================================================================
RCS file: /cvs/src/usr.sbin/cron/funcs.h,v
retrieving revision 1.24
diff -u -p -u -r1.24 funcs.h
--- usr.sbin/cron/funcs.h 4 Nov 2015 12:53:05 -0000 1.24
+++ usr.sbin/cron/funcs.h 8 Nov 2015 14:40:54 -0000
@@ -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);
Index: usr.sbin/cron/job.c
===================================================================
RCS file: /cvs/src/usr.sbin/cron/job.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 job.c
--- usr.sbin/cron/job.c 4 Nov 2015 20:28:17 -0000 1.12
+++ usr.sbin/cron/job.c 8 Nov 2015 14:40:54 -0000
@@ -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);
}
Index: usr.sbin/cron/structs.h
===================================================================
RCS file: /cvs/src/usr.sbin/cron/structs.h,v
retrieving revision 1.6
diff -u -p -u -r1.6 structs.h
--- usr.sbin/cron/structs.h 4 Nov 2015 20:28:17 -0000 1.6
+++ usr.sbin/cron/structs.h 8 Nov 2015 14:40:54 -0000
@@ -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.
- */
Index: usr.sbin/cron/user.c
===================================================================
RCS file: /cvs/src/usr.sbin/cron/user.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 user.c
--- usr.sbin/cron/user.c 4 Nov 2015 20:28:17 -0000 1.16
+++ usr.sbin/cron/user.c 8 Nov 2015 14:40:54 -0000
@@ -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
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
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) {
