On Fri, May 18, 2018 at 11:23:27PM +0200, Martin Ågren wrote:

> diff --git a/unpack-trees.c b/unpack-trees.c
> index 79fd97074e..60293ff536 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -103,6 +103,8 @@ void setup_unpack_trees_porcelain(struct 
> unpack_trees_options *opts,
>       const char **msgs = opts->msgs;
>       const char *msg;
>  
> +     opts->msgs_to_free.strdup_strings = 0;
> +
> [...]
> +void clear_unpack_trees_porcelain(struct unpack_trees_options *opts)
> +{
> +     opts->msgs_to_free.strdup_strings = 1;
> +     string_list_clear(&opts->msgs_to_free, 0);

I like this string_list approach much better, but it's too bad we have
to go through these contortions with the strdup flag to get the memory
ownership right.

If we had a string_list_appendf(), then we could just leave that flag
alone and this:

> @@ -118,8 +120,9 @@ void setup_unpack_trees_porcelain(struct 
> unpack_trees_options *opts,
>                     ? _("Your local changes to the following files would be 
> overwritten by %s:\n%%s"
>                         "Please commit your changes or stash them before you 
> %s.")
>                     : _("Your local changes to the following files would be 
> overwritten by %s:\n%%s");
> -     msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] =
> -             xstrfmt(msg, cmd, cmd);
> +     msg = xstrfmt(msg, cmd, cmd);
> +     msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] = msg;
> +     string_list_append(&opts->msgs_to_free, msg);

would become:

  msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOUPTODATE_FILE] =
        string_list_appendf(&opts->msgs_to_free, msg, cmd, cmd)->string;

I don't know if that's worth it or not (I suspect that there are other
places where appendf would be handy, but I didn't poke around).

-Peff

Reply via email to