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>
This commit is contained in:
René Scharfe 2010-02-08 00:30:20 +01:00 committed by Junio C Hamano
parent 2b26e0e189
commit fe12d8e84f
2 changed files with 26 additions and 18 deletions

View File

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

View File

@ -189,6 +189,16 @@ test_expect_success 'git archive --format=zip with --output' \
'git archive --format=zip --output=d2.zip HEAD && 'git archive --format=zip --output=d2.zip HEAD &&
test_cmp d.zip d2.zip' 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 $UNZIP -v >/dev/null 2>&1
if [ $? -eq 127 ]; then if [ $? -eq 127 ]; then
say "Skipping ZIP tests, because unzip was not found" say "Skipping ZIP tests, because unzip was not found"