archive: simplify archive format guessing

The code to guess an output archive's format consumed any --format
options and built a new one.  Jonathan noticed that it does so in an
unsafe way, risking to overflow the static buffer fmt_opt.

Change the code to keep the existing --format options intact and to only
add a new one if a format could be guessed based on the output file name.
The new option is added as the first one, allowing the existing ones to
overrule it, i.e. explicit --format options given on the command line win
over format guesses, as before.

To simplify the code further, format_from_name() is changed to return the
full --format option, thus no potentially dangerous sprintf() calls are
needed any more.

Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-archive.c b/builtin-archive.c
index 446d6bff30ed47a5cf308310e3f28c5b30648a2c..faf4554d5edbb21dbd043daec669bc684bf056cc 100644 (file)
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -67,7 +67,7 @@ static const char *format_from_name(const char *filename)
                return NULL;
        ext++;
        if (!strcasecmp(ext, "zip"))
-               return "zip";
+               return "--format=zip";
        return NULL;
 }
 
@@ -81,7 +81,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
        const char *exec = "git-upload-archive";
        const char *output = NULL;
        const char *remote = NULL;
-       const char *format = NULL;
+       const char *format_option = NULL;
        struct option local_opts[] = {
                OPT_STRING('o', "output", &output, "file",
                        "write the archive to this file"),
@@ -89,33 +89,31 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
                        "retrieve the archive from remote repository <repo>"),
                OPT_STRING(0, "exec", &exec, "cmd",
                        "path to the remote git-upload-archive command"),
-               OPT_STRING(0, "format", &format, "fmt", "archive format"),
                OPT_END()
        };
-       char fmt_opt[32];
 
        argc = parse_options(argc, argv, prefix, local_opts, NULL,
                             PARSE_OPT_KEEP_ALL);
 
        if (output) {
                create_output_file(output);
-               if (!format)
-                       format = format_from_name(output);
+               format_option = format_from_name(output);
        }
 
-       if (format) {
-               sprintf(fmt_opt, "--format=%s", format);
-               /*
-                * We have enough room in argv[] to muck it in place,
-                * because either --format and/or --output must have
-                * been given on the original command line if we get
-                * to this point, and parse_options() must have eaten
-                * it, i.e. we can add back one element to the array.
-                * But argv[] may contain "--"; we should make it the
-                * first option.
-                */
+       /*
+        * We have enough room in argv[] to muck it in place, because
+        * --output must have been given on the original command line
+        * if we get to this point, and parse_options() must have eaten
+        * it, i.e. we can add back one element to the array.
+        *
+        * We add a fake --format option at the beginning, with the
+        * format inferred from our output filename.  This way explicit
+        * --format options can override it, and the fake option is
+        * inserted before any "--" that might have been given.
+        */
+       if (format_option) {
                memmove(argv + 2, argv + 1, sizeof(*argv) * argc);
-               argv[1] = fmt_opt;
+               argv[1] = format_option;
                argv[++argc] = NULL;
        }
 

diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 0037f63d91a7c11f6599a99e76840a0cdbf4c779..27bfba55bd9631a666ee382b343c3c04af1ad26d 100755 (executable)
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -189,6 +189,16 @@ test_expect_success 'git archive --format=zip with --output' \
     'git archive --format=zip --output=d2.zip HEAD &&
     test_cmp d.zip d2.zip'
 
+test_expect_success 'git archive with --output, inferring format' '
+       git archive --output=d3.zip HEAD &&
+       test_cmp d.zip d3.zip
+'
+
+test_expect_success 'git archive with --output, override inferred format' '
+       git archive --format=tar --output=d4.zip HEAD &&
+       test_cmp b.tar d4.zip
+'
+
 $UNZIP -v >/dev/null 2>&1
 if [ $? -eq 127 ]; then
        say "Skipping ZIP tests, because unzip was not found"