gc: use temporary file for editing crontab

While cron is specified by POSIX, there are a wide variety of
implementations in use.  "git maintenance" assumes that the
"crontab" command can be fed from its standard input the new
contents and the syntax to do so is not to have any filename
argument, as POSIX describes.  However, on FreeBSD, the cron
implementation requires a file name argument: if the user wants to
edit standard input, they must specify "-".

Unfortunately, POSIX systems do not have to interpret "-" on the
command line of crontab as a request to read from the standard
input.  Blindly adding "-" on the command line would not work as a
general solution.

Since POSIX tells us that cron must accept a file name argument, let's
solve this problem by specifying a temporary file instead.  This will
ensure that we work with the vast majority of implementations.

Note that because delete_tempfile closes the file for us, we should not
call fclose here on the handle, since doing so will introduce a double
free.

Reported-by: Renato Botelho <garga@FreeBSD.org>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/gc.c b/builtin/gc.c
index 021e9256ae23561007b1178b3de99341f6c72357..b04f1c552217cb2f0af44bd4fc2cc3bffd916f71 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -2059,6 +2059,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)
        struct child_process crontab_edit = CHILD_PROCESS_INIT;
        FILE *cron_list, *cron_in;
        struct strbuf line = STRBUF_INIT;
+       struct tempfile *tmpedit = NULL;
 
        get_schedule_cmd(&cmd, NULL);
        strvec_split(&crontab_list.args, cmd);
@@ -2073,6 +2074,17 @@ static int crontab_update_schedule(int run_maintenance, int fd)
        /* Ignore exit code, as an empty crontab will return error. */
        finish_command(&crontab_list);
 
+       tmpedit = mks_tempfile_t(".git_cron_edit_tmpXXXXXX");
+       if (!tmpedit) {
+               result = error(_("failed to create crontab temporary file"));
+               goto out;
+       }
+       cron_in = fdopen_tempfile(tmpedit, "w");
+       if (!cron_in) {
+               result = error(_("failed to open temporary file"));
+               goto out;
+       }
+
        /*
         * Read from the .lock file, filtering out the old
         * schedule while appending the new schedule.
@@ -2080,19 +2092,6 @@ static int crontab_update_schedule(int run_maintenance, int fd)
        cron_list = fdopen(fd, "r");
        rewind(cron_list);
 
-       strvec_split(&crontab_edit.args, cmd);
-       crontab_edit.in = -1;
-       crontab_edit.git_cmd = 0;
-
-       if (start_command(&crontab_edit))
-               return error(_("failed to run 'crontab'; your system might not support 'cron'"));
-
-       cron_in = fdopen(crontab_edit.in, "w");
-       if (!cron_in) {
-               result = error(_("failed to open stdin of 'crontab'"));
-               goto done_editing;
-       }
-
        while (!strbuf_getline_lf(&line, cron_list)) {
                if (!in_old_region && !strcmp(line.buf, BEGIN_LINE))
                        in_old_region = 1;
@@ -2126,14 +2125,22 @@ static int crontab_update_schedule(int run_maintenance, int fd)
        }
 
        fflush(cron_in);
-       fclose(cron_in);
-       close(crontab_edit.in);
 
-done_editing:
+       strvec_split(&crontab_edit.args, cmd);
+       strvec_push(&crontab_edit.args, get_tempfile_path(tmpedit));
+       crontab_edit.git_cmd = 0;
+
+       if (start_command(&crontab_edit)) {
+               result = error(_("failed to run 'crontab'; your system might not support 'cron'"));
+               goto out;
+       }
+
        if (finish_command(&crontab_edit))
                result = error(_("'crontab' died"));
        else
                fclose(cron_list);
+out:
+       delete_tempfile(&tmpedit);
        return result;
 }
 

diff --git a/t/helper/test-crontab.c b/t/helper/test-crontab.c
index e7c0137a4779ff005d3b440e1874d570a2d5d797..29425430466692cac0f3aae29e9d0adac0d1805d 100644 (file)
--- a/t/helper/test-crontab.c
+++ b/t/helper/test-crontab.c
@@ -17,8 +17,8 @@ int cmd__crontab(int argc, const char **argv)
                if (!from)
                        return 0;
                to = stdout;
-       } else if (argc == 2) {
-               from = stdin;
+       } else if (argc == 3) {
+               from = fopen(argv[2], "r");
                to = fopen(argv[1], "w");
        } else
                return error("unknown arguments");