Hi Christian,
>From memory, I believe debian already patches cron in one other way to
avoid spinning up laptop drives (I believe it's related to stat()ing
/etc/cron.d to check for updated files?). I believe we should aim to
fix this here, too. I'm attaching a very lightly tested patch. Note
this also fixes a bug in the existing cron "tmpfile" patch regarding
fwrite()ing of the tmpfile buffer to the popen()ed mail command. In
the case of an incomplete write(), the bug causes the beginning of the
buffer to be written again, rather than "resuming" where it left off,
essentially resulting in data loss off the last portion of the buffer.
--- ../orig/do_command.c 2013-06-09 15:37:45.000000000 -0700
+++ ./do_command.c 2013-06-09 16:37:36.000000000 -0700
@@ -127,7 +127,7 @@
user *u;
{
int stdin_pipe[2];
- FILE *tmpout;
+ int stdout_pipe[2];
register char *input_data;
char *usernm, *mailto;
int children = 0;
@@ -191,12 +191,8 @@
/* create a pipe to talk to our future child
*/
pipe(stdin_pipe); /* child's stdin */
- /* child's stdout */
- if ((tmpout = tmpfile()) == NULL) {
- log_it("CRON", getpid(), "error", "create tmpfile");
- exit(ERROR_EXIT);
- }
-
+ pipe(stdout_pipe); /* child's stdout */
+
/* since we are a forked process, we can diddle the command string
* we were passed -- nobody else is going to use it again, right?
*
@@ -284,6 +280,7 @@
* appropriate circumstances.
*/
close(stdin_pipe[WRITE_PIPE]);
+ close(stdout_pipe[READ_PIPE]);
/* grandchild process. make std{in,out} be the ends of
* pipes opened by our daddy; make stderr go to stdout.
@@ -291,14 +288,15 @@
/* Closes are unnecessary -- let dup2() do it */
/* close(STDIN) */; dup2(stdin_pipe[READ_PIPE], STDIN);
- dup2(fileno(tmpout), STDOUT);
/* close(STDERR)*/; dup2(STDOUT, STDERR);
-
/* close the pipe we just dup'ed. The resources will remain.
*/
close(stdin_pipe[READ_PIPE]);
- fclose(tmpout);
+
+ // dup2(fileno(tmpout), STDOUT);
+ dup2(stdout_pipe[WRITE_PIPE], STDOUT);
+ close(stdout_pipe[WRITE_PIPE]);
/* set our login universe. Do this in the grandchild
* so that the child can invoke /usr/lib/sendmail
@@ -404,6 +402,7 @@
* grandchild process...
*/
close(stdin_pipe[READ_PIPE]);
+ close(stdout_pipe[WRITE_PIPE]);
/*
* write, to the pipe connected to child's stdin, any input specified
@@ -465,8 +464,8 @@
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
+ * read output from the grandchild. its stderr has been redirected to
+ * its stdout, which has been redirected to our pipe. if there is any
* output, we'll be mailing it to the user whose crontab this is...
* when the grandchild exits, we'll get EOF.
*/
@@ -506,23 +505,50 @@
}
}
+// Read the output of the command, save it into a tempfile. This
+// doesn't write to a tempfile directly to avoid creating tempfiles
+// and spinning up laptop drives, even when only run-parts is the
+// only, empty job run.
+
+ FILE *tmpout;
+ char buf[1<<9];
+ int ret, remain;
+
+ // if no output, done:
+ if (read(stdout_pipe[READ_PIPE], buf, 1)==0)
+ goto mail_finished;
+
+ // else collect it all in tmpfile:
+ if ((tmpout = tmpfile()) == NULL) {
+ log_it("CRON", getpid(), "error", "create tmpfile");
+ goto mail_finished;
+ }
+
+ putc(*buf, tmpout);
+ for ( ; ret=read(stdout_pipe[READ_PIPE], buf, sizeof(buf)); ) {
+ char *pos;
+ for (pos=buf, remain=ret; remain!=0; ) {
+ int x=fwrite(buf, 1, remain, tmpout);
+ if (x>0) {
+ remain -= ret;
+ pos+=ret;
+ }
+ }
+ }
+
+
// Finally, send any output of the command to the mailer; also, alert
// the user if their job failed. Avoid popening the mailcmd until now
// since sendmail may time out, and to write info about the exit
// status.
-
- long pos;
+ register FILE *mail = NULL;
+ register int bytes = 1;
+
struct stat mcsb;
int statret;
- fseek(tmpout, 0, SEEK_END);
- pos = ftell(tmpout);
- fseek(tmpout, 0, SEEK_SET);
-
Debug(DPROC|DEXT, ("[%d] got %ld bytes data from grandchild tmpfile\n",
getpid(), (long) pos))
- if (pos == 0)
- goto mail_finished;
// get name of recipient.
if (mailto == NULL)
@@ -532,18 +558,13 @@
/* Don't send mail if MAILCMD is not available */
if ((statret = stat(MAILCMD, &mcsb)) != 0) {
- Debug(DPROC|DEXT, ("%s not found, not sending mail\n", MAILCMD))
- if (pos > 0) {
- log_it("CRON", getpid(), "info", "No MTA installed,
discarding output");
- }
+ Debug(DPROC|DEXT, ("%s not found, not sending mail\n",
MAILCMD));
+ log_it("CRON", getpid(), "info", "No MTA installed, discarding
output");
goto mail_finished;
} else {
Debug(DPROC|DEXT, ("%s found, will send mail\n", MAILCMD))
}
- register FILE *mail = NULL;
- register int bytes = 1;
-
register char **env;
char **jobenv = build_env(e->envp);
auto char mailcmd[MAX_COMMAND];
@@ -602,21 +623,15 @@
fputc('\n', mail);
// Append the actual output of the child to the mail
-
- char buf[4096];
- int ret, remain;
-
- while(1) {
- if ((ret = fread(buf, 1, sizeof(buf), tmpout)) == 0)
- break;
- for (remain = ret; remain != 0; ) {
- ret = fwrite(buf, 1, remain, mail);
- if (ret > 0) {
+ rewind(tmpout);
+ for ( ; ret=fread(buf, 1, sizeof(buf), tmpout); ) {
+ char *pos;
+ for (remain=ret, pos=buf; remain!=0; ) {
+ int x=fwrite(buf, 1, remain, mail);
+ if (x>0) {
remain -= ret;
- continue;
+ pos+=ret;
}
- // XXX error
- break;
}
}
