Bug 237538 - [patch] cron: add log suppression and mail suppression for successful runs
Summary: [patch] cron: add log suppression and mail suppression for successful runs
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2019-04-24 18:44 UTC by Naveen Nathan
Modified: 2019-04-25 14:12 UTC (History)
2 users (show)

See Also:


Attachments
Add crontab support for -q (log suppression) and -n (mail suppression for successful run). (11.09 KB, patch)
2019-04-24 18:44 UTC, Naveen Nathan
no flags Details | Diff
Add crontab support for -q (log suppression) and -n (mail suppression for successful run). (11.07 KB, patch)
2019-04-24 19:43 UTC, Naveen Nathan
no flags Details | Diff
Add crontab support for -q (log suppression) and -n (mail suppression for successful run). (11.00 KB, patch)
2019-04-25 01:12 UTC, Naveen Nathan
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Naveen Nathan 2019-04-24 18:44:11 UTC
Created attachment 203978 [details]
Add crontab support for -q (log suppression) and -n (mail suppression for successful run).

I would like to see FreeBSD provide the crontab extensions found in OpenBSD that implement `-n` to suppress mail on successful run and ``q` to suppress logging of command execution.

The `-q` option appears decades old, but the `-n` is relatively new. The original proposal by Job Snijder can be found here [1], and gives very convincing reasons for inclusion in base.

This patch is a nearly identical port of OpenBSD cron for `-q` and `-n` features. It is written to follow existing conventions and style of the existing codebase. I'm also not good at writing manpages but I gave it my best shot.


[1]: https://marc.info/?l=openbsd-tech&m=152874866117948&w=2
Comment 1 Naveen Nathan 2019-04-24 19:00:44 UTC
Comment on attachment 203978 [details]
Add crontab support for -q (log suppression) and -n (mail suppression for successful run).

diff --git a/usr.sbin/cron/cron/cron.h b/usr.sbin/cron/cron/cron.h
index bb4f5287daea..f0f9e88d6b59 100644
--- a/usr.sbin/cron/cron/cron.h
+++ b/usr.sbin/cron/cron/cron.h
@@ -191,6 +191,8 @@ typedef	struct _entry {
 #define	NOT_UNTIL	0x10
 #define	SEC_RES		0x20
 #define	INTERVAL	0x40
+#define	DONT_LOG	0x80
+#define	MAIL_WHEN_ERR	0x100
 	time_t	lastrun;
 } entry;
 
@@ -257,7 +259,7 @@ user		*load_user(int, struct passwd *, char *),
 entry		*load_entry(FILE *, void (*)(char *),
 				 struct passwd *, char **);
 
-FILE		*cron_popen(char *, char *, entry *);
+FILE		*cron_popen(char *, char *, entry *, PID_T *);
 
 
 				/* in the C tradition, we only create
diff --git a/usr.sbin/cron/cron/do_command.c b/usr.sbin/cron/cron/do_command.c
index 13d14e442a27..93e3f4311ebe 100644
--- a/usr.sbin/cron/cron/do_command.c
+++ b/usr.sbin/cron/cron/do_command.c
@@ -41,6 +41,7 @@ static const char rcsid[] =
 static void		child_process(entry *, user *),
 			do_univ(user *);
 
+static WAIT_T		wait_on_child(PID_T, char *);
 
 void
 do_command(e, u)
@@ -94,7 +95,12 @@ child_process(e, u)
 	int		stdin_pipe[2], stdout_pipe[2];
 	register char	*input_data;
 	char		*usernm, *mailto, *mailfrom;
-	int		children = 0;
+	PID_T		jobpid, stdinjob, mailpid;
+
+	register FILE	*mail;
+	register int	bytes = 1;
+	int		status = 0;
+
 # if defined(LOGIN_CAP)
 	struct passwd	*pwd;
 	login_cap_t *lc;
@@ -216,7 +222,8 @@ child_process(e, u)
 
 	/* fork again, this time so we can exec the user's command.
 	 */
-	switch (vfork()) {
+
+	switch (jobpid = vfork()) {
 	case -1:
 		log_it("CRON",getpid(),"error","can't vfork");
 		exit(ERROR_EXIT);
@@ -237,7 +244,7 @@ child_process(e, u)
 		 * the actual user command shell was going to get and the
 		 * PID is part of the log message.
 		 */
-		/*local*/{
+		if ((e->flags & DONT_LOG) == 0) {
 			char *x = mkprints((u_char *)e->cmd, strlen(e->cmd));
 
 			log_it(usernm, getpid(), "CMD", x);
@@ -359,8 +366,6 @@ child_process(e, u)
 		break;
 	}
 
-	children++;
-
 	/* middle process, child of original cron, parent of process running
 	 * the user's command.
 	 */
@@ -384,7 +389,7 @@ child_process(e, u)
 	 * we would block here.  thus we must fork again.
 	 */
 
-	if (*input_data && fork() == 0) {
+	if (*input_data && (stdinjob = fork()) == 0) {
 		register FILE	*out = fdopen(stdin_pipe[WRITE_PIPE], "w");
 		register int	need_newline = FALSE;
 		register int	escaped = FALSE;
@@ -440,8 +445,6 @@ child_process(e, u)
 	 */
 	close(stdin_pipe[WRITE_PIPE]);
 
-	children++;
-
 	/*
 	 * read output from the grandchild.  it's stderr has been redirected to
 	 * it's stdout, which has been redirected to our pipe.  if there is any
@@ -462,10 +465,6 @@ child_process(e, u)
 
 		ch = getc(in);
 		if (ch != EOF) {
-			register FILE	*mail;
-			register int	bytes = 1;
-			int		status = 0;
-
 			Debug(DPROC|DEXT,
 				("[%d] got data (%x:%c) from grandchild\n",
 					getpid(), ch, ch))
@@ -500,7 +499,7 @@ child_process(e, u)
 				hostname[sizeof(hostname) - 1] = '\0';
 				(void) snprintf(mailcmd, sizeof(mailcmd),
 					       MAILARGS, MAILCMD);
-				if (!(mail = cron_popen(mailcmd, "w", e))) {
+				if (!(mail = cron_popen(mailcmd, "w", e, &mailpid))) {
 					warn("%s", MAILCMD);
 					(void) _exit(ERROR_EXIT);
 				}
@@ -538,28 +537,54 @@ child_process(e, u)
 				if (mailto)
 					putc(ch, mail);
 			}
+		} /*if data from grandchild*/
 
-			/* only close pipe if we opened it -- i.e., we're
-			 * mailing...
-			 */
+		Debug(DPROC, ("[%d] got EOF from grandchild\n", getpid()))
 
-			if (mailto) {
-				Debug(DPROC, ("[%d] closing pipe to mail\n",
-					getpid()))
-				/* Note: the pclose will probably see
-				 * the termination of the grandchild
-				 * in addition to the mail process, since
-				 * it (the grandchild) is likely to exit
-				 * after closing its stdout.
-				 */
-				status = cron_pclose(mail);
-			}
+		fclose(in);	/* also closes stdout_pipe[READ_PIPE] */
+	}
+
+	/* wait for children to die.
+	 */
+	if (jobpid > 0) {
+		WAIT_T	waiter;
+
+		waiter = wait_on_child(jobpid, "grandchild command job");
+
+		/* If everything went well, and -n was set, _and_ we have mail,
+		 * we won't be mailing... so shoot the messenger!
+		 */
+		if (WIFEXITED(waiter) && WEXITSTATUS(waiter) == 0
+		    && (e->flags & MAIL_WHEN_ERR) == MAIL_WHEN_ERR
+		    && mailto) {
+			Debug(DPROC, ("[%d] %s executed successful, mail suppressed\n",
+				getpid(), "grandchild command job"))
+			kill(mailpid, SIGKILL);
+			(void)fclose(mail);
+			mailto = NULL;
+		}
+
+
+		/* only close pipe if we opened it -- i.e., we're
+		 * mailing...
+		 */
+
+		if (mailto) {
+			Debug(DPROC, ("[%d] closing pipe to mail\n",
+				getpid()))
+			/* Note: the pclose will probably see
+			 * the termination of the grandchild
+			 * in addition to the mail process, since
+			 * it (the grandchild) is likely to exit
+			 * after closing its stdout.
+			 */
+			status = cron_pclose(mail);
 
 			/* if there was output and we could not mail it,
 			 * log the facts so the poor user can figure out
 			 * what's going on.
 			 */
-			if (mailto && status) {
+			if (status) {
 				char buf[MAX_TEMPSTR];
 
 				snprintf(buf, sizeof(buf),
@@ -568,35 +593,38 @@ child_process(e, u)
 					status);
 				log_it(usernm, getpid(), "MAIL", buf);
 			}
+		}
+	}
 
-		} /*if data from grandchild*/
+	if (*input_data && stdinjob > 0)
+		wait_on_child(stdinjob, "grandchild stdinjob");
+}
 
-		Debug(DPROC, ("[%d] got EOF from grandchild\n", getpid()))
+static WAIT_T
+wait_on_child(PID_T childpid, char *name) {
+	WAIT_T	waiter;
+	PID_T	pid;
 
-		fclose(in);	/* also closes stdout_pipe[READ_PIPE] */
-	}
+	Debug(DPROC, ("[%d] waiting for %s (%d) to finish\n",
+		getpid(), name, childpid))
 
-	/* wait for children to die.
-	 */
-	for (;  children > 0;  children--)
-	{
-		WAIT_T		waiter;
-		PID_T		pid;
-
-		Debug(DPROC, ("[%d] waiting for grandchild #%d to finish\n",
-			getpid(), children))
-		pid = wait(&waiter);
-		if (pid < OK) {
-			Debug(DPROC, ("[%d] no more grandchildren--mail written?\n",
-				getpid()))
-			break;
-		}
-		Debug(DPROC, ("[%d] grandchild #%d finished, status=%04x",
-			getpid(), pid, WEXITSTATUS(waiter)))
-		if (WIFSIGNALED(waiter) && WCOREDUMP(waiter))
-			Debug(DPROC, (", dumped core"))
-		Debug(DPROC, ("\n"))
-	}
+#ifdef POSIX
+  	while ((pid = waitpid(childpid, &waiter, 0)) < 0 && errno == EINTR)
+#else
+  	while ((pid = wait4(childpid, &waiter, 0, NULL)) < 0 && errno == EINTR)
+#endif
+		;
+
+	if (pid < OK)
+		return waiter;
+
+	Debug(DPROC, ("[%d] %s (%d) finished, status=%04x",
+		getpid(), name, pid, WEXITSTATUS(waiter)))
+	if (WIFSIGNALED(waiter) && WCOREDUMP(waiter))
+		Debug(DPROC, (", dumped core"))
+	Debug(DPROC, ("\n"))
+
+	return waiter;
 }
 
 
diff --git a/usr.sbin/cron/cron/popen.c b/usr.sbin/cron/cron/popen.c
index 01e62bf2bab2..73e6e28d748a 100644
--- a/usr.sbin/cron/cron/popen.c
+++ b/usr.sbin/cron/cron/popen.c
@@ -55,9 +55,10 @@ static PID_T *pids;
 static int fds;
 
 FILE *
-cron_popen(program, type, e)
+cron_popen(program, type, e, pidptr)
 	char *program, *type;
 	entry *e;
+	PID_T *pidptr;
 {
 	register char *cp;
 	FILE *iop;
@@ -218,6 +219,9 @@ cron_popen(program, type, e)
 		free((char *)argv[argc]);
 	}
 #endif
+
+	*pidptr = pid;
+
 	return(iop);
 }
 
diff --git a/usr.sbin/cron/crontab/crontab.5 b/usr.sbin/cron/crontab/crontab.5
index 8988574f3745..354aba3dd8fb 100644
--- a/usr.sbin/cron/crontab/crontab.5
+++ b/usr.sbin/cron/crontab/crontab.5
@@ -198,7 +198,8 @@ Ranges or
 lists of names are not allowed.
 .Pp
 The ``sixth'' field (the rest of the line) specifies the command to be
-run.
+run. One or more command options may precede the command to modify
+processing behavior.
 The entire command portion of the line, up to a newline or %
 character, will be executed by
 .Pa /bin/sh
@@ -211,6 +212,22 @@ Percent-signs (%) in the command, unless escaped with backslash
 after the first % will be sent to the command as standard
 input.
 .Pp
+The following command options can be supplied:
+.Bl -tag -width Ds
+.It Fl n
+No mail is sent after a successful run.
+The execution output will only be mailed if the command exits with a non-zero
+exit code.
+The
+.Fl n
+option is an attempt to cure potentially copious volumes of mail coming from
+.Xr cron 8 .
+.It Fl q
+Execution will not be logged.
+.El
+
+Duplicate options are not allowed.
+.Pp
 Note: The day of a command's execution can be specified by two
 fields \(em day of month, and day of week.
 If both fields are
@@ -261,6 +278,10 @@ SHELL=/bin/sh
 # mail any output to `paul', no matter whose crontab this is
 MAILTO=paul
 #
+# run every minute, suppress logging
+* * * * *       -q date
+# run every minute, suppress logging and mail
+* * * * *       -q -n echo "no-op"
 # run five minutes after midnight, every day
 5 0 * * *       $HOME/bin/daily.job >> $HOME/tmp/out 2>&1
 # run at 2:15pm on the first of every month -- output mailed to paul
@@ -314,6 +335,12 @@ All of the
 .Sq @
 directives that can appear in place of the first five fields
 are extensions.
+.Pp
+Command processing can be modified using command options. The
+.Sq -q
+option suppresses logging. The
+.Sq -n
+option does not mail on successful run.
 .Sh AUTHORS
 .An Paul Vixie Aq Mt paul@vix.com
 .Sh BUGS
diff --git a/usr.sbin/cron/lib/entry.c b/usr.sbin/cron/lib/entry.c
index a8ec3ae8b568..a81a4ee78c30 100644
--- a/usr.sbin/cron/lib/entry.c
+++ b/usr.sbin/cron/lib/entry.c
@@ -35,7 +35,8 @@ static const char rcsid[] =
 
 typedef	enum ecode {
 	e_none, e_minute, e_hour, e_dom, e_month, e_dow,
-	e_cmd, e_timespec, e_username, e_group, e_mem
+	e_cmd, e_timespec, e_username, e_group, e_option,
+	e_mem
 #ifdef LOGIN_CAP
 	, e_class
 #endif
@@ -58,6 +59,7 @@ static char *ecodes[] =
 		"bad time specifier",
 		"bad username",
 		"bad group name",
+		"bad option",
 		"out of memory",
 #ifdef LOGIN_CAP
 		"bad class name",
@@ -429,6 +431,53 @@ load_entry(file, error_func, pw, envp)
 	}
 #endif
 
+	Debug(DPARS, ("load_entry()...checking for command options\n"))
+
+	ch = get_char(file);
+
+	while (ch == '-') {
+		Debug(DPARS|DEXT, ("load_entry()...expecting option\n"))
+		switch(ch = get_char(file)) {
+		case 'n':
+			Debug(DPARS|DEXT, ("load_entry()...got MAIL_WHEN_ERR ('n') option\n"))
+			/* only allow the user to set the option once */
+			if ((e->flags & MAIL_WHEN_ERR) == MAIL_WHEN_ERR) {
+				Debug(DPARS|DEXT, ("load_entry()...duplicate MAIL_WHEN_ERR ('n') option\n"))
+				ecode = e_option;
+				goto eof;
+			}
+			e->flags |= MAIL_WHEN_ERR;
+			break;
+		case 'q':
+			Debug(DPARS|DEXT, ("load_entry()...got DONT_LOG ('q') option\n"))
+			/* only allow the user to set the option once */
+			if ((e->flags & DONT_LOG) == DONT_LOG) {
+				Debug(DPARS|DEXT, ("load_entry()...duplicate DONT_LOG ('q') option\n"))
+				ecode = e_option;
+				goto eof;
+			}
+			e->flags |= DONT_LOG;
+			break;
+		default:
+			Debug(DPARS|DEXT, ("load_entry()...invalid option '%c'\n", ch))
+			ecode = e_option;
+			goto eof;
+		}
+		ch = get_char(file);
+		if (ch!='\t' && ch!=' ') {
+			ecode = e_option;
+			goto eof;
+		}
+
+		Skip_Blanks(ch, file)
+		if (ch == EOF || ch == '\n') {
+			ecode = e_cmd;
+			goto eof;
+		}
+	}
+
+	unget_char(ch, file);
+
 	Debug(DPARS, ("load_entry()...about to parse command\n"))
 
 	/* Everything up to the next \n or EOF is part of the command...
Comment 2 Naveen Nathan 2019-04-24 19:02:11 UTC
Comment on attachment 203978 [details]
Add crontab support for -q (log suppression) and -n (mail suppression for successful run).

diff --git a/usr.sbin/cron/cron/cron.h b/usr.sbin/cron/cron/cron.h
index bb4f5287daea..f0f9e88d6b59 100644
--- a/usr.sbin/cron/cron/cron.h
+++ b/usr.sbin/cron/cron/cron.h
@@ -191,6 +191,8 @@ typedef	struct _entry {
 #define	NOT_UNTIL	0x10
 #define	SEC_RES		0x20
 #define	INTERVAL	0x40
+#define	DONT_LOG	0x80
+#define	MAIL_WHEN_ERR	0x100
 	time_t	lastrun;
 } entry;
 
@@ -257,7 +259,7 @@ user		*load_user(int, struct passwd *, char *),
 entry		*load_entry(FILE *, void (*)(char *),
 				 struct passwd *, char **);
 
-FILE		*cron_popen(char *, char *, entry *);
+FILE		*cron_popen(char *, char *, entry *, PID_T *);
 
 
 				/* in the C tradition, we only create
diff --git a/usr.sbin/cron/cron/do_command.c b/usr.sbin/cron/cron/do_command.c
index 13d14e442a27..93e3f4311ebe 100644
--- a/usr.sbin/cron/cron/do_command.c
+++ b/usr.sbin/cron/cron/do_command.c
@@ -41,6 +41,7 @@ static const char rcsid[] =
 static void		child_process(entry *, user *),
 			do_univ(user *);
 
+static WAIT_T		wait_on_child(PID_T, char *);
 
 void
 do_command(e, u)
@@ -94,7 +95,12 @@ child_process(e, u)
 	int		stdin_pipe[2], stdout_pipe[2];
 	register char	*input_data;
 	char		*usernm, *mailto, *mailfrom;
-	int		children = 0;
+	PID_T		jobpid, stdinjob, mailpid;
+
+	register FILE	*mail;
+	register int	bytes = 1;
+	int		status = 0;
+
 # if defined(LOGIN_CAP)
 	struct passwd	*pwd;
 	login_cap_t *lc;
@@ -216,7 +222,8 @@ child_process(e, u)
 
 	/* fork again, this time so we can exec the user's command.
 	 */
-	switch (vfork()) {
+
+	switch (jobpid = vfork()) {
 	case -1:
 		log_it("CRON",getpid(),"error","can't vfork");
 		exit(ERROR_EXIT);
@@ -237,7 +244,7 @@ child_process(e, u)
 		 * the actual user command shell was going to get and the
 		 * PID is part of the log message.
 		 */
-		/*local*/{
+		if ((e->flags & DONT_LOG) == 0) {
 			char *x = mkprints((u_char *)e->cmd, strlen(e->cmd));
 
 			log_it(usernm, getpid(), "CMD", x);
@@ -359,8 +366,6 @@ child_process(e, u)
 		break;
 	}
 
-	children++;
-
 	/* middle process, child of original cron, parent of process running
 	 * the user's command.
 	 */
@@ -384,7 +389,7 @@ child_process(e, u)
 	 * we would block here.  thus we must fork again.
 	 */
 
-	if (*input_data && fork() == 0) {
+	if (*input_data && (stdinjob = fork()) == 0) {
 		register FILE	*out = fdopen(stdin_pipe[WRITE_PIPE], "w");
 		register int	need_newline = FALSE;
 		register int	escaped = FALSE;
@@ -440,8 +445,6 @@ child_process(e, u)
 	 */
 	close(stdin_pipe[WRITE_PIPE]);
 
-	children++;
-
 	/*
 	 * read output from the grandchild.  it's stderr has been redirected to
 	 * it's stdout, which has been redirected to our pipe.  if there is any
@@ -462,10 +465,6 @@ child_process(e, u)
 
 		ch = getc(in);
 		if (ch != EOF) {
-			register FILE	*mail;
-			register int	bytes = 1;
-			int		status = 0;
-
 			Debug(DPROC|DEXT,
 				("[%d] got data (%x:%c) from grandchild\n",
 					getpid(), ch, ch))
@@ -500,7 +499,7 @@ child_process(e, u)
 				hostname[sizeof(hostname) - 1] = '\0';
 				(void) snprintf(mailcmd, sizeof(mailcmd),
 					       MAILARGS, MAILCMD);
-				if (!(mail = cron_popen(mailcmd, "w", e))) {
+				if (!(mail = cron_popen(mailcmd, "w", e, &mailpid))) {
 					warn("%s", MAILCMD);
 					(void) _exit(ERROR_EXIT);
 				}
@@ -538,28 +537,54 @@ child_process(e, u)
 				if (mailto)
 					putc(ch, mail);
 			}
+		} /*if data from grandchild*/
 
-			/* only close pipe if we opened it -- i.e., we're
-			 * mailing...
-			 */
+		Debug(DPROC, ("[%d] got EOF from grandchild\n", getpid()))
 
-			if (mailto) {
-				Debug(DPROC, ("[%d] closing pipe to mail\n",
-					getpid()))
-				/* Note: the pclose will probably see
-				 * the termination of the grandchild
-				 * in addition to the mail process, since
-				 * it (the grandchild) is likely to exit
-				 * after closing its stdout.
-				 */
-				status = cron_pclose(mail);
-			}
+		fclose(in);	/* also closes stdout_pipe[READ_PIPE] */
+	}
+
+	/* wait for children to die.
+	 */
+	if (jobpid > 0) {
+		WAIT_T	waiter;
+
+		waiter = wait_on_child(jobpid, "grandchild command job");
+
+		/* If everything went well, and -n was set, _and_ we have mail,
+		 * we won't be mailing... so shoot the messenger!
+		 */
+		if (WIFEXITED(waiter) && WEXITSTATUS(waiter) == 0
+		    && (e->flags & MAIL_WHEN_ERR) == MAIL_WHEN_ERR
+		    && mailto) {
+			Debug(DPROC, ("[%d] %s executed successful, mail suppressed\n",
+				getpid(), "grandchild command job"))
+			kill(mailpid, SIGKILL);
+			(void)fclose(mail);
+			mailto = NULL;
+		}
+
+
+		/* only close pipe if we opened it -- i.e., we're
+		 * mailing...
+		 */
+
+		if (mailto) {
+			Debug(DPROC, ("[%d] closing pipe to mail\n",
+				getpid()))
+			/* Note: the pclose will probably see
+			 * the termination of the grandchild
+			 * in addition to the mail process, since
+			 * it (the grandchild) is likely to exit
+			 * after closing its stdout.
+			 */
+			status = cron_pclose(mail);
 
 			/* if there was output and we could not mail it,
 			 * log the facts so the poor user can figure out
 			 * what's going on.
 			 */
-			if (mailto && status) {
+			if (status) {
 				char buf[MAX_TEMPSTR];
 
 				snprintf(buf, sizeof(buf),
@@ -568,35 +593,38 @@ child_process(e, u)
 					status);
 				log_it(usernm, getpid(), "MAIL", buf);
 			}
+		}
+	}
 
-		} /*if data from grandchild*/
+	if (*input_data && stdinjob > 0)
+		wait_on_child(stdinjob, "grandchild stdinjob");
+}
 
-		Debug(DPROC, ("[%d] got EOF from grandchild\n", getpid()))
+static WAIT_T
+wait_on_child(PID_T childpid, char *name) {
+	WAIT_T	waiter;
+	PID_T	pid;
 
-		fclose(in);	/* also closes stdout_pipe[READ_PIPE] */
-	}
+	Debug(DPROC, ("[%d] waiting for %s (%d) to finish\n",
+		getpid(), name, childpid))
 
-	/* wait for children to die.
-	 */
-	for (;  children > 0;  children--)
-	{
-		WAIT_T		waiter;
-		PID_T		pid;
-
-		Debug(DPROC, ("[%d] waiting for grandchild #%d to finish\n",
-			getpid(), children))
-		pid = wait(&waiter);
-		if (pid < OK) {
-			Debug(DPROC, ("[%d] no more grandchildren--mail written?\n",
-				getpid()))
-			break;
-		}
-		Debug(DPROC, ("[%d] grandchild #%d finished, status=%04x",
-			getpid(), pid, WEXITSTATUS(waiter)))
-		if (WIFSIGNALED(waiter) && WCOREDUMP(waiter))
-			Debug(DPROC, (", dumped core"))
-		Debug(DPROC, ("\n"))
-	}
+#ifdef POSIX
+  	while ((pid = waitpid(childpid, &waiter, 0)) < 0 && errno == EINTR)
+#else
+  	while ((pid = wait4(childpid, &waiter, 0, NULL)) < 0 && errno == EINTR)
+#endif
+		;
+
+	if (pid < OK)
+		return waiter;
+
+	Debug(DPROC, ("[%d] %s (%d) finished, status=%04x",
+		getpid(), name, pid, WEXITSTATUS(waiter)))
+	if (WIFSIGNALED(waiter) && WCOREDUMP(waiter))
+		Debug(DPROC, (", dumped core"))
+	Debug(DPROC, ("\n"))
+
+	return waiter;
 }
 
 
diff --git a/usr.sbin/cron/cron/popen.c b/usr.sbin/cron/cron/popen.c
index 01e62bf2bab2..73e6e28d748a 100644
--- a/usr.sbin/cron/cron/popen.c
+++ b/usr.sbin/cron/cron/popen.c
@@ -55,9 +55,10 @@ static PID_T *pids;
 static int fds;
 
 FILE *
-cron_popen(program, type, e)
+cron_popen(program, type, e, pidptr)
 	char *program, *type;
 	entry *e;
+	PID_T *pidptr;
 {
 	register char *cp;
 	FILE *iop;
@@ -218,6 +219,9 @@ cron_popen(program, type, e)
 		free((char *)argv[argc]);
 	}
 #endif
+
+	*pidptr = pid;
+
 	return(iop);
 }
 
diff --git a/usr.sbin/cron/crontab/crontab.5 b/usr.sbin/cron/crontab/crontab.5
index 8988574f3745..354aba3dd8fb 100644
--- a/usr.sbin/cron/crontab/crontab.5
+++ b/usr.sbin/cron/crontab/crontab.5
@@ -198,7 +198,8 @@ Ranges or
 lists of names are not allowed.
 .Pp
 The ``sixth'' field (the rest of the line) specifies the command to be
-run.
+run. One or more command options may precede the command to modify
+processing behavior.
 The entire command portion of the line, up to a newline or %
 character, will be executed by
 .Pa /bin/sh
@@ -211,6 +212,22 @@ Percent-signs (%) in the command, unless escaped with backslash
 after the first % will be sent to the command as standard
 input.
 .Pp
+The following command options can be supplied:
+.Bl -tag -width Ds
+.It Fl n
+No mail is sent after a successful run.
+The execution output will only be mailed if the command exits with a non-zero
+exit code.
+The
+.Fl n
+option is an attempt to cure potentially copious volumes of mail coming from
+.Xr cron 8 .
+.It Fl q
+Execution will not be logged.
+.El
+
+Duplicate options are not allowed.
+.Pp
 Note: The day of a command's execution can be specified by two
 fields \(em day of month, and day of week.
 If both fields are
@@ -261,6 +278,10 @@ SHELL=/bin/sh
 # mail any output to `paul', no matter whose crontab this is
 MAILTO=paul
 #
+# run every minute, suppress logging
+* * * * *       -q date
+# run every minute, suppress logging and mail
+* * * * *       -q -n echo "no-op"
 # run five minutes after midnight, every day
 5 0 * * *       $HOME/bin/daily.job >> $HOME/tmp/out 2>&1
 # run at 2:15pm on the first of every month -- output mailed to paul
@@ -314,6 +335,12 @@ All of the
 .Sq @
 directives that can appear in place of the first five fields
 are extensions.
+.Pp
+Command processing can be modified using command options. The
+.Sq -q
+option suppresses logging. The
+.Sq -n
+option does not mail on successful run.
 .Sh AUTHORS
 .An Paul Vixie Aq Mt paul@vix.com
 .Sh BUGS
diff --git a/usr.sbin/cron/lib/entry.c b/usr.sbin/cron/lib/entry.c
index a8ec3ae8b568..a81a4ee78c30 100644
--- a/usr.sbin/cron/lib/entry.c
+++ b/usr.sbin/cron/lib/entry.c
@@ -35,7 +35,8 @@ static const char rcsid[] =
 
 typedef	enum ecode {
 	e_none, e_minute, e_hour, e_dom, e_month, e_dow,
-	e_cmd, e_timespec, e_username, e_group, e_mem
+	e_cmd, e_timespec, e_username, e_group, e_option,
+	e_mem
 #ifdef LOGIN_CAP
 	, e_class
 #endif
@@ -58,6 +59,7 @@ static char *ecodes[] =
 		"bad time specifier",
 		"bad username",
 		"bad group name",
+		"bad option",
 		"out of memory",
 #ifdef LOGIN_CAP
 		"bad class name",
@@ -429,6 +431,53 @@ load_entry(file, error_func, pw, envp)
 	}
 #endif
 
+	Debug(DPARS, ("load_entry()...checking for command options\n"))
+
+	ch = get_char(file);
+
+	while (ch == '-') {
+		Debug(DPARS|DEXT, ("load_entry()...expecting option\n"))
+		switch(ch = get_char(file)) {
+		case 'n':
+			Debug(DPARS|DEXT, ("load_entry()...got MAIL_WHEN_ERR ('n') option\n"))
+			/* only allow the user to set the option once */
+			if ((e->flags & MAIL_WHEN_ERR) == MAIL_WHEN_ERR) {
+				Debug(DPARS|DEXT, ("load_entry()...duplicate MAIL_WHEN_ERR ('n') option\n"))
+				ecode = e_option;
+				goto eof;
+			}
+			e->flags |= MAIL_WHEN_ERR;
+			break;
+		case 'q':
+			Debug(DPARS|DEXT, ("load_entry()...got DONT_LOG ('q') option\n"))
+			/* only allow the user to set the option once */
+			if ((e->flags & DONT_LOG) == DONT_LOG) {
+				Debug(DPARS|DEXT, ("load_entry()...duplicate DONT_LOG ('q') option\n"))
+				ecode = e_option;
+				goto eof;
+			}
+			e->flags |= DONT_LOG;
+			break;
+		default:
+			Debug(DPARS|DEXT, ("load_entry()...invalid option '%c'\n", ch))
+			ecode = e_option;
+			goto eof;
+		}
+		ch = get_char(file);
+		if (ch!='\t' && ch!=' ') {
+			ecode = e_option;
+			goto eof;
+		}
+
+		Skip_Blanks(ch, file)
+		if (ch == EOF || ch == '\n') {
+			ecode = e_cmd;
+			goto eof;
+		}
+	}
+
+	unget_char(ch, file);
+
 	Debug(DPARS, ("load_entry()...about to parse command\n"))
 
 	/* Everything up to the next \n or EOF is part of the command...
Comment 3 Naveen Nathan 2019-04-24 19:37:51 UTC
Comment on attachment 203978 [details]
Add crontab support for -q (log suppression) and -n (mail suppression for successful run).

diff --git a/usr.sbin/cron/cron/cron.h b/usr.sbin/cron/cron/cron.h
index bb4f5287daea..f0f9e88d6b59 100644
--- a/usr.sbin/cron/cron/cron.h
+++ b/usr.sbin/cron/cron/cron.h
@@ -191,6 +191,8 @@ typedef	struct _entry {
 #define	NOT_UNTIL	0x10
 #define	SEC_RES		0x20
 #define	INTERVAL	0x40
+#define	DONT_LOG	0x80
+#define	MAIL_WHEN_ERR	0x100
 	time_t	lastrun;
 } entry;
 
@@ -257,7 +259,7 @@ user		*load_user(int, struct passwd *, char *),
 entry		*load_entry(FILE *, void (*)(char *),
 				 struct passwd *, char **);
 
-FILE		*cron_popen(char *, char *, entry *);
+FILE		*cron_popen(char *, char *, entry *, PID_T *);
 
 
 				/* in the C tradition, we only create
diff --git a/usr.sbin/cron/cron/do_command.c b/usr.sbin/cron/cron/do_command.c
index 13d14e442a27..b54297c02cb2 100644
--- a/usr.sbin/cron/cron/do_command.c
+++ b/usr.sbin/cron/cron/do_command.c
@@ -41,6 +41,7 @@ static const char rcsid[] =
 static void		child_process(entry *, user *),
 			do_univ(user *);
 
+static WAIT_T		wait_on_child(PID_T, char *);
 
 void
 do_command(e, u)
@@ -94,7 +95,12 @@ child_process(e, u)
 	int		stdin_pipe[2], stdout_pipe[2];
 	register char	*input_data;
 	char		*usernm, *mailto, *mailfrom;
-	int		children = 0;
+	PID_T		jobpid, stdinjob, mailpid;
+
+	register FILE	*mail;
+	register int	bytes = 1;
+	int		status = 0;
+
 # if defined(LOGIN_CAP)
 	struct passwd	*pwd;
 	login_cap_t *lc;
@@ -216,7 +222,8 @@ child_process(e, u)
 
 	/* fork again, this time so we can exec the user's command.
 	 */
-	switch (vfork()) {
+
+	switch (jobpid = vfork()) {
 	case -1:
 		log_it("CRON",getpid(),"error","can't vfork");
 		exit(ERROR_EXIT);
@@ -237,7 +244,7 @@ child_process(e, u)
 		 * the actual user command shell was going to get and the
 		 * PID is part of the log message.
 		 */
-		/*local*/{
+		if ((e->flags & DONT_LOG) == 0) {
 			char *x = mkprints((u_char *)e->cmd, strlen(e->cmd));
 
 			log_it(usernm, getpid(), "CMD", x);
@@ -359,8 +366,6 @@ child_process(e, u)
 		break;
 	}
 
-	children++;
-
 	/* middle process, child of original cron, parent of process running
 	 * the user's command.
 	 */
@@ -384,7 +389,9 @@ child_process(e, u)
 	 * we would block here.  thus we must fork again.
 	 */
 
-	if (*input_data && fork() == 0) {
+	stdinjob = 0;
+
+	if (*input_data && (stdinjob = fork()) == 0) {
 		register FILE	*out = fdopen(stdin_pipe[WRITE_PIPE], "w");
 		register int	need_newline = FALSE;
 		register int	escaped = FALSE;
@@ -440,8 +447,6 @@ child_process(e, u)
 	 */
 	close(stdin_pipe[WRITE_PIPE]);
 
-	children++;
-
 	/*
 	 * read output from the grandchild.  it's stderr has been redirected to
 	 * it's stdout, which has been redirected to our pipe.  if there is any
@@ -462,10 +467,6 @@ child_process(e, u)
 
 		ch = getc(in);
 		if (ch != EOF) {
-			register FILE	*mail;
-			register int	bytes = 1;
-			int		status = 0;
-
 			Debug(DPROC|DEXT,
 				("[%d] got data (%x:%c) from grandchild\n",
 					getpid(), ch, ch))
@@ -500,7 +501,7 @@ child_process(e, u)
 				hostname[sizeof(hostname) - 1] = '\0';
 				(void) snprintf(mailcmd, sizeof(mailcmd),
 					       MAILARGS, MAILCMD);
-				if (!(mail = cron_popen(mailcmd, "w", e))) {
+				if (!(mail = cron_popen(mailcmd, "w", e, &mailpid))) {
 					warn("%s", MAILCMD);
 					(void) _exit(ERROR_EXIT);
 				}
@@ -538,28 +539,54 @@ child_process(e, u)
 				if (mailto)
 					putc(ch, mail);
 			}
+		} /*if data from grandchild*/
 
-			/* only close pipe if we opened it -- i.e., we're
-			 * mailing...
-			 */
+		Debug(DPROC, ("[%d] got EOF from grandchild\n", getpid()))
+
+		fclose(in);	/* also closes stdout_pipe[READ_PIPE] */
+	}
+
+	/* wait for children to die.
+	 */
+	if (jobpid > 0) {
+		WAIT_T	waiter;
+
+		waiter = wait_on_child(jobpid, "grandchild command job");
+
+		/* If everything went well, and -n was set, _and_ we have mail,
+		 * we won't be mailing... so shoot the messenger!
+		 */
+		if (WIFEXITED(waiter) && WEXITSTATUS(waiter) == 0
+		    && (e->flags & MAIL_WHEN_ERR) == MAIL_WHEN_ERR
+		    && mailto) {
+			Debug(DPROC, ("[%d] %s executed successful, mail suppressed\n",
+				getpid(), "grandchild command job"))
+			kill(mailpid, SIGKILL);
+			(void)fclose(mail);
+			mailto = NULL;
+		}
 
-			if (mailto) {
-				Debug(DPROC, ("[%d] closing pipe to mail\n",
-					getpid()))
-				/* Note: the pclose will probably see
-				 * the termination of the grandchild
-				 * in addition to the mail process, since
-				 * it (the grandchild) is likely to exit
-				 * after closing its stdout.
-				 */
-				status = cron_pclose(mail);
-			}
+
+		/* only close pipe if we opened it -- i.e., we're
+		 * mailing...
+		 */
+
+		if (mailto) {
+			Debug(DPROC, ("[%d] closing pipe to mail\n",
+				getpid()))
+			/* Note: the pclose will probably see
+			 * the termination of the grandchild
+			 * in addition to the mail process, since
+			 * it (the grandchild) is likely to exit
+			 * after closing its stdout.
+			 */
+			status = cron_pclose(mail);
 
 			/* if there was output and we could not mail it,
 			 * log the facts so the poor user can figure out
 			 * what's going on.
 			 */
-			if (mailto && status) {
+			if (status) {
 				char buf[MAX_TEMPSTR];
 
 				snprintf(buf, sizeof(buf),
@@ -568,35 +595,38 @@ child_process(e, u)
 					status);
 				log_it(usernm, getpid(), "MAIL", buf);
 			}
+		}
+	}
 
-		} /*if data from grandchild*/
+	if (stdinjob > 0)
+		wait_on_child(stdinjob, "grandchild stdinjob");
+}
 
-		Debug(DPROC, ("[%d] got EOF from grandchild\n", getpid()))
+static WAIT_T
+wait_on_child(PID_T childpid, char *name) {
+	WAIT_T	waiter;
+	PID_T	pid;
 
-		fclose(in);	/* also closes stdout_pipe[READ_PIPE] */
-	}
+	Debug(DPROC, ("[%d] waiting for %s (%d) to finish\n",
+		getpid(), name, childpid))
 
-	/* wait for children to die.
-	 */
-	for (;  children > 0;  children--)
-	{
-		WAIT_T		waiter;
-		PID_T		pid;
-
-		Debug(DPROC, ("[%d] waiting for grandchild #%d to finish\n",
-			getpid(), children))
-		pid = wait(&waiter);
-		if (pid < OK) {
-			Debug(DPROC, ("[%d] no more grandchildren--mail written?\n",
-				getpid()))
-			break;
-		}
-		Debug(DPROC, ("[%d] grandchild #%d finished, status=%04x",
-			getpid(), pid, WEXITSTATUS(waiter)))
-		if (WIFSIGNALED(waiter) && WCOREDUMP(waiter))
-			Debug(DPROC, (", dumped core"))
-		Debug(DPROC, ("\n"))
-	}
+#ifdef POSIX
+  	while ((pid = waitpid(childpid, &waiter, 0)) < 0 && errno == EINTR)
+#else
+  	while ((pid = wait4(childpid, &waiter, 0, NULL)) < 0 && errno == EINTR)
+#endif
+		;
+
+	if (pid < OK)
+		return waiter;
+
+	Debug(DPROC, ("[%d] %s (%d) finished, status=%04x",
+		getpid(), name, pid, WEXITSTATUS(waiter)))
+	if (WIFSIGNALED(waiter) && WCOREDUMP(waiter))
+		Debug(DPROC, (", dumped core"))
+	Debug(DPROC, ("\n"))
+
+	return waiter;
 }
 
 
diff --git a/usr.sbin/cron/cron/popen.c b/usr.sbin/cron/cron/popen.c
index 01e62bf2bab2..73e6e28d748a 100644
--- a/usr.sbin/cron/cron/popen.c
+++ b/usr.sbin/cron/cron/popen.c
@@ -55,9 +55,10 @@ static PID_T *pids;
 static int fds;
 
 FILE *
-cron_popen(program, type, e)
+cron_popen(program, type, e, pidptr)
 	char *program, *type;
 	entry *e;
+	PID_T *pidptr;
 {
 	register char *cp;
 	FILE *iop;
@@ -218,6 +219,9 @@ cron_popen(program, type, e)
 		free((char *)argv[argc]);
 	}
 #endif
+
+	*pidptr = pid;
+
 	return(iop);
 }
 
diff --git a/usr.sbin/cron/crontab/crontab.5 b/usr.sbin/cron/crontab/crontab.5
index 8988574f3745..354aba3dd8fb 100644
--- a/usr.sbin/cron/crontab/crontab.5
+++ b/usr.sbin/cron/crontab/crontab.5
@@ -198,7 +198,8 @@ Ranges or
 lists of names are not allowed.
 .Pp
 The ``sixth'' field (the rest of the line) specifies the command to be
-run.
+run. One or more command options may precede the command to modify
+processing behavior.
 The entire command portion of the line, up to a newline or %
 character, will be executed by
 .Pa /bin/sh
@@ -211,6 +212,22 @@ Percent-signs (%) in the command, unless escaped with backslash
 after the first % will be sent to the command as standard
 input.
 .Pp
+The following command options can be supplied:
+.Bl -tag -width Ds
+.It Fl n
+No mail is sent after a successful run.
+The execution output will only be mailed if the command exits with a non-zero
+exit code.
+The
+.Fl n
+option is an attempt to cure potentially copious volumes of mail coming from
+.Xr cron 8 .
+.It Fl q
+Execution will not be logged.
+.El
+
+Duplicate options are not allowed.
+.Pp
 Note: The day of a command's execution can be specified by two
 fields \(em day of month, and day of week.
 If both fields are
@@ -261,6 +278,10 @@ SHELL=/bin/sh
 # mail any output to `paul', no matter whose crontab this is
 MAILTO=paul
 #
+# run every minute, suppress logging
+* * * * *       -q date
+# run every minute, suppress logging and mail
+* * * * *       -q -n echo "no-op"
 # run five minutes after midnight, every day
 5 0 * * *       $HOME/bin/daily.job >> $HOME/tmp/out 2>&1
 # run at 2:15pm on the first of every month -- output mailed to paul
@@ -314,6 +335,12 @@ All of the
 .Sq @
 directives that can appear in place of the first five fields
 are extensions.
+.Pp
+Command processing can be modified using command options. The
+.Sq -q
+option suppresses logging. The
+.Sq -n
+option does not mail on successful run.
 .Sh AUTHORS
 .An Paul Vixie Aq Mt paul@vix.com
 .Sh BUGS
diff --git a/usr.sbin/cron/lib/entry.c b/usr.sbin/cron/lib/entry.c
index a8ec3ae8b568..a81a4ee78c30 100644
--- a/usr.sbin/cron/lib/entry.c
+++ b/usr.sbin/cron/lib/entry.c
@@ -35,7 +35,8 @@ static const char rcsid[] =
 
 typedef	enum ecode {
 	e_none, e_minute, e_hour, e_dom, e_month, e_dow,
-	e_cmd, e_timespec, e_username, e_group, e_mem
+	e_cmd, e_timespec, e_username, e_group, e_option,
+	e_mem
 #ifdef LOGIN_CAP
 	, e_class
 #endif
@@ -58,6 +59,7 @@ static char *ecodes[] =
 		"bad time specifier",
 		"bad username",
 		"bad group name",
+		"bad option",
 		"out of memory",
 #ifdef LOGIN_CAP
 		"bad class name",
@@ -429,6 +431,53 @@ load_entry(file, error_func, pw, envp)
 	}
 #endif
 
+	Debug(DPARS, ("load_entry()...checking for command options\n"))
+
+	ch = get_char(file);
+
+	while (ch == '-') {
+		Debug(DPARS|DEXT, ("load_entry()...expecting option\n"))
+		switch(ch = get_char(file)) {
+		case 'n':
+			Debug(DPARS|DEXT, ("load_entry()...got MAIL_WHEN_ERR ('n') option\n"))
+			/* only allow the user to set the option once */
+			if ((e->flags & MAIL_WHEN_ERR) == MAIL_WHEN_ERR) {
+				Debug(DPARS|DEXT, ("load_entry()...duplicate MAIL_WHEN_ERR ('n') option\n"))
+				ecode = e_option;
+				goto eof;
+			}
+			e->flags |= MAIL_WHEN_ERR;
+			break;
+		case 'q':
+			Debug(DPARS|DEXT, ("load_entry()...got DONT_LOG ('q') option\n"))
+			/* only allow the user to set the option once */
+			if ((e->flags & DONT_LOG) == DONT_LOG) {
+				Debug(DPARS|DEXT, ("load_entry()...duplicate DONT_LOG ('q') option\n"))
+				ecode = e_option;
+				goto eof;
+			}
+			e->flags |= DONT_LOG;
+			break;
+		default:
+			Debug(DPARS|DEXT, ("load_entry()...invalid option '%c'\n", ch))
+			ecode = e_option;
+			goto eof;
+		}
+		ch = get_char(file);
+		if (ch!='\t' && ch!=' ') {
+			ecode = e_option;
+			goto eof;
+		}
+
+		Skip_Blanks(ch, file)
+		if (ch == EOF || ch == '\n') {
+			ecode = e_cmd;
+			goto eof;
+		}
+	}
+
+	unget_char(ch, file);
+
 	Debug(DPARS, ("load_entry()...about to parse command\n"))
 
 	/* Everything up to the next \n or EOF is part of the command...
Comment 4 Naveen Nathan 2019-04-24 19:43:30 UTC
Created attachment 203979 [details]
Add crontab support for -q (log suppression) and -n (mail suppression for successful run).

Two fixes to the original patch:

1. stdinjob wasn't being reaped because of a bad condition.

2. proc debug output when reaping the command and stdin job grandchilds is now consistent with how it was done originally.
Comment 5 Naveen Nathan 2019-04-25 01:12:52 UTC
Created attachment 203994 [details]
Add crontab support for -q (log suppression) and -n (mail suppression for successful run).

Added a more useful example for -n in crontab(5).
Comment 6 Naveen Nathan 2019-04-25 14:12:21 UTC
Currently getting patch reviewed on phabricator: https://reviews.freebsd.org/D20046