Bug 237538
| Summary: | [patch] cron: add log suppression and mail suppression for successful runs | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Naveen Nathan <freebsd> |
| Component: | bin | Assignee: | Kyle Evans <kevans> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | CC: | dmitry.wagin, freebsd, kevans, sigsys |
| Priority: | --- | Keywords: | patch |
| Version: | CURRENT | Flags: | kevans:
mfc-stable12+
kevans: mfc-stable11+ |
| Hardware: | Any | ||
| OS: | Any | ||
| Attachments: | |||
|
Description
Naveen Nathan
2019-04-24 18:44: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 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 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... 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.
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).
Currently getting patch reviewed on phabricator: https://reviews.freebsd.org/D20046 A commit references this bug: Author: kevans Date: Wed Sep 25 02:37:42 UTC 2019 New revision: 352668 URL: https://svnweb.freebsd.org/changeset/base/352668 Log: cron: add log suppression and mail suppression for successful runs This commit adds two new extensions to crontab, ported from OpenBSD: - -n: suppress mail on succesful run - -q: suppress logging of command execution The -q option appears decades old, but -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. Example usage: # should only send email, but won't show up in log * * * * * -q date # should not send email * * * * * -n date # should not send email or log * * * * * -n -q date # should send email because of ping failure * * * * * -n -q ping -c 1 5.5.5.5 [1]: https://marc.info/?l=openbsd-tech&m=152874866117948&w=2 PR: 237538 Submitted by: Naveen Nathan <freebsd_t.lastninja.net> Reviewed by: bcr (manpages) MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D20046 Changes: head/usr.sbin/cron/cron/cron.h head/usr.sbin/cron/cron/do_command.c head/usr.sbin/cron/cron/popen.c head/usr.sbin/cron/crontab/crontab.5 head/usr.sbin/cron/lib/entry.c Committed... apologies for the delay in review. Will MFC to stable/1{1,2}.
A commit references this bug: Author: kevans Date: Sun Oct 6 03:56:05 UTC 2019 New revision: 353134 URL: https://svnweb.freebsd.org/changeset/base/353134 Log: MFC r352668: cron: log suppression and mail suppression for successful runs This commit adds two new extensions to crontab, ported from OpenBSD: - -n: suppress mail on succesful run - -q: suppress logging of command execution The -q option appears decades old, but -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. Example usage: # should only send email, but won't show up in log * * * * * -q date # should not send email * * * * * -n date # should not send email or log * * * * * -n -q date # should send email because of ping failure * * * * * -n -q ping -c 1 5.5.5.5 [1]: https://marc.info/?l=openbsd-tech&m=152874866117948&w=2 PR: 237538 Relnotes: yes Changes: _U stable/11/ stable/11/usr.sbin/cron/cron/cron.h stable/11/usr.sbin/cron/cron/do_command.c stable/11/usr.sbin/cron/cron/popen.c stable/11/usr.sbin/cron/crontab/crontab.5 stable/11/usr.sbin/cron/lib/entry.c _U stable/12/ stable/12/usr.sbin/cron/cron/cron.h stable/12/usr.sbin/cron/cron/do_command.c stable/12/usr.sbin/cron/cron/popen.c stable/12/usr.sbin/cron/crontab/crontab.5 stable/12/usr.sbin/cron/lib/entry.c MFC'd to all supported branches; thanks! Сhanged behavior. man 5 crontab If MAILTO is defined but empty (MAILTO=""), no mail will be sent. I had an old MAILTO="" in crontab, after update logs are filled with such messages: Nov 13 14:11:00 bsd1 cron[11723]: (operator) MAIL (mailed 1 byte of output but got status 0xffffffff ) ... Changing setting MAILTO="" -> MAILTO= removes the messages. |
