[Bug 237538] [patch] cron: add log suppression and mail suppression for successful runs

bugzilla-noreply at freebsd.org bugzilla-noreply at freebsd.org
Wed Apr 24 19:37:53 UTC 2019


https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=237538

--- Comment #3 from Naveen Nathan <freebsd at t.lastninja.net> ---
Comment on attachment 203978
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=203978
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 at 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...

-- 
You are receiving this mail because:
You are the assignee for the bug.


More information about the freebsd-bugs mailing list