parse_opt_string_list: stop allocating new strings

The parse_opt_string_list callback is basically a thin
wrapper to string_list_append() any string options we get.
However, it calls:

string_list_append(v, xstrdup(arg));

which duplicates the option value. This is wrong for two
reasons:

1. If the string list has strdup_strings set, then we are
making an extra copy, which is simply leaked.

2. If the string list does not have strdup_strings set,
then we pass memory ownership to the string list, but
it does not realize this. If we later call
string_list_clear(), which can happen if "--no-foo" is
passed, then we will leak all of the existing entries.

Instead, we should just pass the argument straight to
string_list_append, and it can decide whether to copy or not
based on its strdup_strings flag.

It's possible that some (buggy) caller could be relying on
this extra copy (e.g., because it parses some options from
an allocated argv array and then frees the array), but it's
not likely. For one, we generally only use parse_options on
the argv given to us in main(). And two, such a caller is
broken anyway, because other option types like OPT_STRING()
do not make such a copy.  This patch brings us in line with
them.

Noticed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/parse-options-cb.c b/parse-options-cb.c
index 5ab6ed6b0875ff703e3d55abbd0e9d8b0aa1a3c7..4e8290181ff718a48296d6da551c182722691630 100644 (file)
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -127,7 +127,7 @@ int parse_opt_string_list(const struct option *opt, const char *arg, int unset)
        if (!arg)
                return -1;
 
-       string_list_append(v, xstrdup(arg));
+       string_list_append(v, arg);
        return 0;
 }