various changes: remove some magic numbers, break some long lines, remove unwanted USED(argc,argv), fix the `don't go mad' test (was off by a factor of 60), permit jobs to be initiated each minute, not each two minutes, and initiate them starting in the first second of the minute. tested and it seems to work. Reference: /n/sources/patch/applied/cron-accuracy Date: Mon May 8 06:38:11 CES 2006 Signed-off-by: geoff@collyer.net --- /sys/src/cmd/auth/cron.c Mon May 8 06:32:33 2006 +++ /sys/src/cmd/auth/cron.c Mon May 8 06:32:30 2006 @@ -7,12 +7,18 @@ char CRONLOG[] = "cron"; +enum { + Minute = 60, + Hour = 60 * Minute, + Day = 24 * Hour, +}; + typedef struct Job Job; typedef struct Time Time; typedef struct User User; struct Time{ /* bit masks for each valid time */ - ulong min; /* actually 1 bit for every 2 min */ + uvlong min; ulong hour; ulong mday; ulong wday; @@ -45,7 +51,7 @@ void readalljobs(void); Job *readjobs(char*, User*); int getname(char**); -ulong gettime(int, int); +uvlong gettime(int, int); int gettok(int, int); void initcap(void); void pushtok(void); @@ -67,7 +73,7 @@ Job *j; Tm tm; Time t; - ulong now, last, x; + ulong now, last, nowsecs, future; int i; debug = 0; @@ -81,7 +87,6 @@ default: usage(); }ARGEND - USED(argc, argv); if(debug){ readalljobs(); @@ -102,30 +107,36 @@ argv0 = "cron"; srand(getpid()*time(0)); - last = time(0) / 60; + last = time(0) / Minute; for(;;){ readalljobs(); - now = time(0) / 60; - if ((now-last) > (60*60*24)) /* don't go mad */ - last = now-(60*60*24); - for(; last <= now; last += 2){ - tm = *localtime(last*60); - t.min = 1 << tm.min/2; + now = time(0) / Minute; + if (now-last > Day/Minute) /* don't go mad */ + last = now - Day/Minute; /* just execute 1 day's jobs */ + for(; last <= now; last++){ + tm = *localtime(last*Minute); + t.min = 1ULL << tm.min; t.hour = 1 << tm.hour; t.wday = 1 << tm.wday; t.mday = 1 << tm.mday; - t.mon = 1 << (tm.mon + 1); + t.mon = 1 << (tm.mon + 1); for(i = 0; i < nuser; i++) for(j = users[i].jobs; j; j = j->next) - if(j->time.min & t.min && j->time.hour & t.hour + if(j->time.min & t.min + && j->time.hour & t.hour && j->time.wday & t.wday && j->time.mday & t.mday && j->time.mon & t.mon) rexec(&users[i], j); } - x = time(0) / 60; - if(x - now < 2) - sleep((2 - (x - now))*60*1000); + /* + * if we're not there yet, sleep until (now+1)*Minute, + * which synchronises with minute roll-over as a side-effect. + */ + future = (now + 1) * Minute; + nowsecs = time(0); + if (nowsecs < future) + sleep((future - nowsecs)*1000); } /* not reached */ } @@ -172,7 +183,8 @@ if(strcmp(d[i].name, "log") == 0) continue; if(strcmp(d[i].name, d[i].uid) != 0){ - syslog(1, CRONLOG, "cron for %s owned by %s\n", d[i].name, d[i].uid); + syslog(1, CRONLOG, "cron for %s owned by %s\n", + d[i].name, d[i].uid); continue; } u = newuser(d[i].name); @@ -219,12 +231,13 @@ if(*savec == '#' || *savec == '\0') continue; if(strlen(savec) > 1024){ - syslog(0, CRONLOG, "%s: line %d: line too long", user->name, line); + syslog(0, CRONLOG, "%s: line %d: line too long", + user->name, line); continue; } j = emalloc(sizeof *j); - if((j->time.min = gettime(0, 59)) - && (j->time.hour = gettime(0, 23)) + j->time.min = gettime(0, 59); + if(j->time.min && (j->time.hour = gettime(0, 23)) && (j->time.mday = gettime(1, 31)) && (j->time.mon = gettime(1, 12)) && (j->time.wday = gettime(0, 6)) @@ -234,7 +247,8 @@ j->next = jobs; jobs = j; }else{ - syslog(0, CRONLOG, "%s: line %d: syntax error", user->name, line); + syslog(0, CRONLOG, "%s: line %d: syntax error", + user->name, line); free(j); } } @@ -251,12 +265,12 @@ for(i = 0; i < nuser; i++){ print("user %s\n", users[i].name); - for(j = users[i].jobs; j; j = j->next){ + for(j = users[i].jobs; j; j = j->next) if(!mkcmd(j->cmd, buf, sizeof buf)) - print("\tbad job %s on host %s\n", j->cmd, j->host); + print("\tbad job %s on host %s\n", + j->cmd, j->host); else print("\tjob %s on host %s\n", buf, j->host); - } } } @@ -321,7 +335,7 @@ } /* - * return the next time range in the file: + * return the next time range (as a bit vector) in the file: * times: '*' * | range * range: number @@ -329,21 +343,21 @@ * | range ',' range * a return of zero means a syntax error was discovered */ -ulong +uvlong gettime(int min, int max) { - ulong n, m, e; + uvlong n, m, e; if(gettok(min, max) == '*') - return ~0; + return ~0ULL; n = 0; while(tok == '1'){ - m = 1 << lexval; + m = 1ULL << lexval; n |= m; if(gettok(0, 0) == '-'){ if(gettok(lexval, max) != '1') return 0; - e = 1 << lexval; + e = 1ULL << lexval; for( ; m <= e; m <<= 1) n |= m; gettok(min, max); @@ -379,8 +393,6 @@ lexval = strtoul(savec, &savec, 10); if(lexval < min || lexval > max) return tok = 0; - if(max > 32) - lexval /= 2; /* yuk: correct min by / 2 */ return tok = '1'; case '*': case '-': case ',': savec++; @@ -476,23 +488,25 @@ _exits(0); } - alarm(2*60*1000); + alarm(2*Minute*1000); fd = call(j->host); if(fd < 0){ - if(fd == -2){ - syslog(0, CRONLOG, "%s: dangerous host %s", user->name, j->host); - } + if(fd == -2) + syslog(0, CRONLOG, "%s: dangerous host %s", + user->name, j->host); syslog(0, CRONLOG, "%s: can't call %s: %r", user->name, j->host); _exits(0); } syslog(0, CRONLOG, "%s: called %s on %s", user->name, j->cmd, j->host); if(becomeuser(user->name) < 0){ - syslog(0, CRONLOG, "%s: can't change uid for %s on %s: %r", user->name, j->cmd, j->host); + syslog(0, CRONLOG, "%s: can't change uid for %s on %s: %r", + user->name, j->cmd, j->host); _exits(0); } ai = auth_proxy(fd, nil, "proto=p9any role=client"); if(ai == nil){ - syslog(0, CRONLOG, "%s: can't authenticate for %s on %s: %r", user->name, j->cmd, j->host); + syslog(0, CRONLOG, "%s: can't authenticate for %s on %s: %r", + user->name, j->cmd, j->host); _exits(0); } syslog(0, CRONLOG, "%s: authenticated %s on %s", user->name, j->cmd, j->host);