parse-options: free previous value of `OPTION_FILENAME`

The `OPTION_FILENAME` option always assigns either an allocated string
or `NULL` to the value. In case it is passed multiple times it does not
know to free the previous value though, which causes a memory leak.

Refactor the function to always free the previous value. None of the
sites where this option is used pass a string constant, so this change
is safe.

While at it, fix the argument of `fix_filename()` to be a string
constant. The only reason why it's not is because we use it as an
in-out-parameter, where the input is a constant and the output is not.
This is weird and unnecessary, as we can just return the result instead
of using the parameter for this.

This leak is being hit in t7621, but plugging it alone does not make the
test suite pass.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/parse-options.c b/parse-options.c
index 30b9e68f8ac85df1c6700d7a512eef618988b06c..33bfba0ed4a0eaad1bb3ce61876d6c01b6d2c6aa 100644 (file)
--- a/parse-options.c
+++ b/parse-options.c
@@ -60,12 +60,12 @@ static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
        return 0;
 }
 
-static void fix_filename(const char *prefix, char **file)
+static char *fix_filename(const char *prefix, const char *file)
 {
        if (!file || !*file)
-               ; /* leave as NULL */
+               return NULL;
        else
-               *file = prefix_filename_except_for_dash(prefix, *file);
+               return prefix_filename_except_for_dash(prefix, file);
 }
 
 static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
@@ -129,18 +129,24 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
                return 0;
 
        case OPTION_FILENAME:
+       {
+               const char *value;
+
+               FREE_AND_NULL(*(char **)opt->value);
+
                err = 0;
+
                if (unset)
-                       *(const char **)opt->value = NULL;
+                       value = NULL;
                else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
-                       *(const char **)opt->value = (const char *)opt->defval;
+                       value = (const char *) opt->defval;
                else
-                       err = get_arg(p, opt, flags, (const char **)opt->value);
+                       err = get_arg(p, opt, flags, &value);
 
                if (!err)
-                       fix_filename(p->prefix, (char **)opt->value);
+                       *(char **)opt->value = fix_filename(p->prefix, value);
                return err;
-
+       }
        case OPTION_CALLBACK:
        {
                const char *p_arg = NULL;