]> git.ipfire.org Git - thirdparty/git.git/commit - parse-options-cb.c
parse_opt_string_list: stop allocating new strings
authorJeff King <peff@peff.net>
Mon, 13 Jun 2016 05:39:12 +0000 (01:39 -0400)
committerJunio C Hamano <gitster@pobox.com>
Mon, 13 Jun 2016 17:33:08 +0000 (10:33 -0700)
commit7a7a517a2f6264a893ed47f8daf02cb221aca67c
treea0f2fae7af9403b5369789450153ef7488215c10
parente46579643d56162299b1756b70d418005351b256
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>
parse-options-cb.c