NetBSD Problem Report #45453

From www@NetBSD.org  Wed Oct 12 02:12:55 2011
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 3618063D4F6
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 12 Oct 2011 02:12:55 +0000 (UTC)
Message-Id: <20111012021254.1EB9B63B955@www.NetBSD.org>
Date: Wed, 12 Oct 2011 02:12:54 +0000 (UTC)
From: mspo@netbsd.org
Reply-To: mspo@netbsd.org
To: gnats-bugs@NetBSD.org
Subject: add /etc/cron.d functionality
X-Send-Pr-Version: www-1.0

>Number:         45453
>Category:       bin
>Synopsis:       add /etc/cron.d functionality
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Wed Oct 12 02:15:00 +0000 2011
>Last-Modified:  Fri Oct 14 07:10:03 +0000 2011
>Originator:     matthew sporleder
>Release:        5.1
>Organization:
mspo.com
>Environment:
NetBSD vc136-15.vc.panix.com 5.1 NetBSD 5.1 (PANIX-VC) #2: Mon Aug  8 22:10:38 EDT 2011  root@juggler.panix.com:/misc/obj/misc/devel/netbsd/5.1/src/sys/arch/amd64/compile/PANIX-VC amd64

>Description:
I would like to see supported added for /etc/cron.d to our cron, as found in linux.
>How-To-Repeat:

>Fix:
jmcneill found this patch from redhat:
http://netbsd.org/~jmcneill/vixie-cron-4.1-_6_rh_crond.patch

We would also need to define RH_CROND_DIR in pathnames.h

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
@@ -36,7 +36,7 @@

 void
 load_database(cron_db *old_db) {
-	struct stat statbuf, syscron_stat;
+	struct stat statbuf, syscron_stat, crond_stat;
 	cron_db new_db;
 	DIR_T *dp;
 	DIR *dir;
@@ -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);
+	}
+
 	/* track system crontab file
 	 */
 	if (stat(SYSCRONTAB, &syscron_stat) < OK)
@@ -65,7 +70,9 @@
 	 * so is guaranteed to be different than the stat() mtime the first
 	 * time this function is called.
 	 */
-	if (old_db->mtime == TMAX(statbuf.st_mtime, syscron_stat.st_mtime)) {
+	if (old_db->mtime == TMAX(crond_stat.st_mtime,
+				  TMAX(statbuf.st_mtime, syscron_stat.st_mtime))
+	   ){
 		Debug(DLOAD, ("[%ld] spool dir mtime unch, no load needed.\n",
 			      (long)getpid()))
 		return;
@@ -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;
+
+		/* avoid file names beginning with ".".  this is good
+		 * because we would otherwise waste two guaranteed calls
+		 * to getpwnam() for . and .., and there shouldn't be 
+		 * hidden files in here anyway
+		 */
+		if (dp->d_name[0] == '.')
+			continue;
+
+		/* ignore files starting with # and ending with ~ */
+		if (dp->d_name[0] == '#')
+			continue;
+		
+		len = strlen(dp->d_name);
+
+		if (len >= sizeof fname)
+			continue;	/* XXX log? */
+
+		if ((len > 0) && (dp->d_name[len - 1] == '~'))
+			continue;
+
+		(void) strcpy(fname, dp->d_name);
+		
+		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;
+
+		if (!glue_strings(tabname, sizeof tabname, RH_CROND_DIR, fname, '/'))
+			continue;	/* XXX log? */
+
+		process_crontab("root", "*system*", tabname,
+				&crond_stat, &new_db, old_db);
+	}
+	closedir(dir);
+
 	/* we used to keep this dir open all the time, for the sake of
 	 * efficiency.  however, we need to close it in every fork, and
 	 * we fork a lot more often than the mtime of the dir changes.

>Audit-Trail:
From: Matthew Mondor <mm_lists@pulsar-zone.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45453: add /etc/cron.d functionality
Date: Tue, 11 Oct 2011 22:58:38 -0400

 On Wed, 12 Oct 2011 02:15:01 +0000 (UTC)
 mspo@netbsd.org wrote:

 > I would like to see supported added for /etc/cron.d to our cron, as found in linux.

 Thanks for the diff,

 Would the main use of cron.d be for packages to be able to add cron
 events?

 In which case, packages could come with defaults provided
 in /etc/pkg/cron.d/ which the admin would manually copy to /etc/cron.d/
 if wanted, just like for rc.d scripts?  Currently packages that need
 scheduled events can only tell the admin to install one via a message.
 It would probably be necessary for those files to specify under which
 user privileges the script should run as.

 Another alternative might be to instead use /usr/pkg/etc/cron.d/ and
 have cron also load those, but I guess that this might be too automatic
 for some admin security expectations.

 I guess that tech-pkg could be a good place to discuss this issue
 further.

 If it's for use by the admin only, I don't object but also don't
 understand the requirement...

 Thanks,
 -- 
 Matt

From: "Matthew Sporleder (mspo.com)" <mspo@mspo.com>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, mspo@netbsd.org
Subject: Re: bin/45453: add /etc/cron.d functionality
Date: Tue, 11 Oct 2011 23:20:58 -0400

 >  Would the main use of cron.d be for packages to be able to add cron
 >  events?
 >  
 >  In which case, packages could come with defaults provided
 >  in /etc/pkg/cron.d/ which the admin would manually copy to /etc/cron.d/
 >  if wanted, just like for rc.d scripts?  Currently packages that need
 >  scheduled events can only tell the admin to install one via a message.
 >  It would probably be necessary for those files to specify under which
 >  user privileges the script should run as.
 >  

 That is definitely a use-case that I had not considered.

 My primary use-case is that it's easier for me to script/automate flat files
 than write parsers for the existing crontabs.

 It's a convenience and has a side benefit of linux compatibility.

From: Jukka Ruohonen <jruohonen@iki.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45453: add /etc/cron.d functionality
Date: Wed, 12 Oct 2011 09:07:57 +0300

 On Tue, Oct 11, 2011 at 11:20:58PM -0400, Matthew Sporleder (mspo.com) wrote:
 > My primary use-case is that it's easier for me to script/automate flat files
 > than write parsers for the existing crontabs.

 Why can't you execute the flat files from daily.conf(5) etc.?

From: Takahiro Kambe <taca@back-street.net>
To: gnats-bugs@NetBSD.org, mspo@netbsd.org
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>

From: Hauke Fath <hauke@Espresso.Rhein-Neckar.DE>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@NetBSD.org, mspo@NetBSD.org
Subject: Re: bin/45453: add /etc/cron.d functionality
Date: Wed, 12 Oct 2011 09:17:05 +0200

 At 3:25 Uhr +0000 12.10.2011, Matthew Sporleder (mspo.com) wrote:
 > My primary use-case is that it's easier for me to script/automate flat files
 > than write parsers for the existing crontabs.
 >
 > It's a convenience and has a side benefit of linux compatibility.

 Linux has cron entries in five different places - I don't quite see a
 benefit in being compatible here...

 My 0.02 EUR.

 hauke

 --
 "It's never straight up and down"     (DEVO)


From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45453: add /etc/cron.d functionality
Date: Wed, 12 Oct 2011 10:02:53 +0100

 On Wed, Oct 12, 2011 at 03:25:03AM +0000, Matthew Sporleder (mspo.com) wrote:
 >  
 >  My primary use-case is that it's easier for me to script/automate flat files
 >  than write parsers for the existing crontabs.

 Write a cron job to update crontab from cron.d/* :-)

 	David

 -- 
 David Laight: david@l8s.co.uk

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45453: add /etc/cron.d functionality
Date: Fri, 14 Oct 2011 05:23:22 +0000

 On Wed, Oct 12, 2011 at 06:35:02AM +0000, Takahiro Kambe wrote:
  >> +	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.)

 Rather than blacklist some set of extensions (and as you note, the
 ones it has seem totally wrong) it ought to accept only files named
 e.g. *.cron. Much safer that way.

 -- 
 David A. Holland
 dholland@netbsd.org

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, mspo@netbsd.org
Subject: Re: bin/45453: add /etc/cron.d functionality
Date: Fri, 14 Oct 2011 08:03:25 +0100

 On Fri, Oct 14, 2011 at 05:25:01AM +0000, David Holland wrote:
 > The following reply was made to PR bin/45453; it has been noted by GNATS.
 > 
 > From: David Holland <dholland-bugs@netbsd.org>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: bin/45453: add /etc/cron.d functionality
 > Date: Fri, 14 Oct 2011 05:23:22 +0000
 > 
 >  On Wed, Oct 12, 2011 at 06:35:02AM +0000, Takahiro Kambe wrote:
 >   >> +	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.)
 >  
 >  Rather than blacklist some set of extensions (and as you note, the
 >  ones it has seem totally wrong) it ought to accept only files named
 >  e.g. *.cron. Much safer that way.

 And, if you are going to do the above, it would be better to have
 something like:
 	static const char *bad_suffixes[] = { "rpmsave", ..., 0};
 	const char **bp, *dot = strrchr(fname, '.');
 	if (dot) {
 		for (bp = bad_suffixes; *bp; bp++)
 			if (strcmp(dot + 1, *bp) == 0)
 				return FALSE;
 	}
 	return TRUE;

 But, a positive match is better!
 I hope the code skipped direcories?

 	David

 -- 
 David Laight: david@l8s.co.uk

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.