Re: `git bundle create -` may not write to `stdout`

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> The most directed fix is this:
>
> diff --git a/builtin/bundle.c b/builtin/bundle.c
> index acceef6200..145b814f48 100644
> --- a/builtin/bundle.c
> +++ b/builtin/bundle.c
> @@ -59,7 +59,9 @@ static int parse_options_cmd_bundle(int argc,
>  			     PARSE_OPT_STOP_AT_NON_OPTION);
>  	if (!argc)
>  		usage_msg_opt(_("need a <file> argument"), usagestr, options);
> -	*bundle_file = prefix_filename(prefix, argv[0]);
> +	*bundle_file = strcmp(argv[0], "-") ?
> +		       prefix_filename(prefix, argv[0]) :
> +		       xstrdup(argv[0]);
>  	return argc;
>  }

This fell thru cracks, it seems.

OPT_FILENAME() needs to do exactly this, and has its own helper
function in parse-options.c::fix_filename(), but its memory
ownership rules is somewhat screwed (it was perfectly fine in the
original context of "we parse the bounded number of options the user
would give us from the command line, and giving more than one
instance of these last-one-wins option may leak but we do not
care---if you do not like leaking, don't give duplicates", but with
automated leak checking that does not care about such nuances, it
will trigger unnecessary errors), and cannot be reused here.

Here is your "most directed fix" packaged into a call to a helper
function.  Given that we may want to slim the cache.h header, it may
not want to be declared there, but for now, its declaration sits
next to prefix_filename().


diff --git c/abspath.c w/abspath.c
index 39e06b5848..b2fd9ff321 100644
--- c/abspath.c
+++ w/abspath.c
@@ -280,3 +280,10 @@ char *prefix_filename(const char *pfx, const char *arg)
 #endif
 	return strbuf_detach(&path, NULL);
 }
+
+char *prefix_filename_except_for_dash(const char *pfx, const char *arg)
+{
+	if (!strcmp(arg, "-"))
+		return xstrdup(arg);
+	return prefix_filename(pfx, arg);
+}
diff --git c/builtin/bundle.c w/builtin/bundle.c
index acceef6200..3056dbeee3 100644
--- c/builtin/bundle.c
+++ w/builtin/bundle.c
@@ -59,7 +59,7 @@ static int parse_options_cmd_bundle(int argc,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	if (!argc)
 		usage_msg_opt(_("need a <file> argument"), usagestr, options);
-	*bundle_file = prefix_filename(prefix, argv[0]);
+	*bundle_file = prefix_filename_except_for_dash(prefix, argv[0]);
 	return argc;
 }
 
diff --git c/cache.h w/cache.h
index 12789903e8..38867de41a 100644
--- c/cache.h
+++ w/cache.h
@@ -638,6 +638,9 @@ char *prefix_path_gently(const char *prefix, int len, int *remaining, const char
  */
 char *prefix_filename(const char *prefix, const char *path);
 
+/* Likewise, but path=="-" always yields "-" */
+char *prefix_filename_except_for_dash(const char *prefix, const char *path);
+
 int check_filename(const char *prefix, const char *name);
 void verify_filename(const char *prefix,
 		     const char *name,
diff --git c/parse-options.c w/parse-options.c
index fd4743293f..bd1ed906a8 100644
--- c/parse-options.c
+++ w/parse-options.c
@@ -59,6 +59,10 @@ static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
 	return 0;
 }
 
+/*
+ * NEEDSWORK: fix memory ownership rules around here and then use
+ * prefix_filename_except_for_dash() helper.
+ */
 static void fix_filename(const char *prefix, const char **file)
 {
 	if (!file || !*file || !prefix || is_absolute_path(*file)
diff --git c/t/t6020-bundle-misc.sh w/t/t6020-bundle-misc.sh
index 7d40994991..d14f7cea91 100755
--- c/t/t6020-bundle-misc.sh
+++ w/t/t6020-bundle-misc.sh
@@ -606,4 +606,15 @@ test_expect_success 'verify catches unreachable, broken prerequisites' '
 	)
 '
 
+test_expect_success 'send a bundle to standard output' '
+	git bundle create - --all HEAD >bundle-one &&
+	mkdir -p down &&
+	git -C down bundle create - --all HEAD >bundle-two &&
+	git bundle verify bundle-one &&
+	git bundle verify bundle-two &&
+	git ls-remote bundle-one >expect &&
+	git ls-remote bundle-two >actual &&
+	test_cmp expect actual
+'
+
 test_done