Re: bin/45453: add /etc/cron.d functionality

[Takahiro Kambe <taca%back-street.net@localhost>]

The following reply was made to PR bin/45453; it has been noted by GNATS.

From: Takahiro Kambe <taca%back-street.net@localhost>
To: gnats-bugs%NetBSD.org@localhost, mspo%netbsd.org@localhost
Cc: 
Subject: Re: bin/45453: add /etc/cron.d functionality
Date: Wed, 12 Oct 2011 15:31:18 +0900 (JST)

 A few comments.
 
 I don't like very much about /etc/cron.d style.  But if we introduce it:
 
 > Here is the patch:
 > --- vixie-cron-4.1-rh/database.c.rh_crond    2004-07-21 10:16:43.000000000 
 > -0400
 > +++ vixie-cron-4.1-rh/database.c     2004-07-22 09:47:20.000000000 -0400
 ...
 > @@ -53,6 +53,11 @@
 >              (void) exit(ERROR_EXIT);
 >      }
 >  
 > +    if (stat(RH_CROND_DIR, &crond_stat) < OK) {
 > +            log_it("CRON", getpid(), "STAT FAILED", RH_CROND_DIR);
 > +            (void) exit(ERROR_EXIT);
 > +    }
 > +
 If there wasn't RH_CROND_DIR, it should be safely ignored for backward
 compatibility.
 
 > @@ -76,13 +83,61 @@
 >       * actually changed.  Whatever is left in the old database when
 >       * we're done is chaff -- crontabs that disappeared.
 >       */
 > -    new_db.mtime = TMAX(statbuf.st_mtime, syscron_stat.st_mtime);
 > +    new_db.mtime = TMAX(crond_stat.st_mtime,
 > +                        TMAX(statbuf.st_mtime, syscron_stat.st_mtime));
 >      new_db.head = new_db.tail = NULL;
 >  
 >      if (syscron_stat.st_mtime)
 >              process_crontab("root", NULL, SYSCRONTAB, &syscron_stat,
 >                              &new_db, old_db);
 >  
 > +    if (!(dir = opendir(RH_CROND_DIR))) {
 > +            log_it("CRON", getpid(), "OPENDIR FAILED", RH_CROND_DIR);
 > +            (void) exit(ERROR_EXIT);
 > +    }
 > +
 > +    while (NULL != (dp = readdir(dir))) {
 > +            char    fname[MAXNAMLEN+1],
 > +                    tabname[MAXNAMLEN+1];
 > +            size_t len;
 I don't think it is good idea that opendir/readdir each time.  cron(8)
 could be cache each /etc/cron.d/file and its mtime in memory.
 
 > +            if ((len > 8) && (strncmp(fname + len - 8, ".rpmsave", 8) == 0))
 > +                    continue;
 > +            if ((len > 8) && (strncmp(fname + len - 8, ".rpmorig", 8) == 0))
 > +                    continue;
 > +            if ((len > 7) && (strncmp(fname + len - 7, ".rpmnew", 7) == 0))
 > +                    continue;
 We don't hardcode such suffixes but give cron(8) to some excluding
 patterns.  (Above is too rpmism, too.)
 
 -- 
 Takahiro Kambe <taca%back-street.net@localhost>