Junio C Hamano <gits...@pobox.com> writes:

> I have a feeling that argv_array might be a better fit for the
> purpose of keeping track of to_free[] strings in the context of this
> series.  Moving away from string_list would allow us to sidestep the
> storage ownership issues the API has, and we do not need the .util
> thing string_list gives us (which is one distinct advantage string_list
> has over argv_array, if the application needs that feature).
>
> We would need to make _pushf() and friends return "const char *" if
> we go that route to make the resulting API more useful, though.

... and redoing the 4/4 patch using argv_array_pushf() makes the
result look like this, which does not look too bad.

-- >8 --
From: Junio C Hamano <gits...@pobox.com>
Subject: [PATCH] unpack_trees_options: keep track of owned messages with 
argv_array

Instead of the string_list API, which is overly flexible and require
callers to be careful about memory ownership issues, use the
argv_array API that always takes ownership to redo the earlier
commit.

Signed-off-by: Junio C Hamano <gits...@pobox.com>
---
 unpack-trees.c | 16 ++++++----------
 unpack-trees.h |  4 ++--
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 86046b987a..b28f0c6e9d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1,5 +1,6 @@
 #define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
+#include "argv-array.h"
 #include "repository.h"
 #include "config.h"
 #include "dir.h"
@@ -103,11 +104,7 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
        const char **msgs = opts->msgs;
        const char *msg;
 
-       /*
-        * As we add strings using `...appendf()`, this does not matter,
-        * but when we clear the string list, we want them to be freed.
-        */
-       opts->msgs_to_free.strdup_strings = 1;
+       argv_array_init(&opts->msgs_to_free);
 
        if (!strcmp(cmd, "checkout"))
                msg = advice_commit_before_merge
@@ -125,7 +122,7 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
                          "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] =
-               string_list_appendf(&opts->msgs_to_free, msg, cmd, cmd)->string;
+               argv_array_pushf(&opts->msgs_to_free, msg, cmd, cmd);
 
        msgs[ERROR_NOT_UPTODATE_DIR] =
                _("Updating the following directories would lose untracked 
files in them:\n%s");
@@ -146,7 +143,7 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
                          "Please move or remove them before you %s.")
                      : _("The following untracked working tree files would be 
removed by %s:\n%%s");
        msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] =
-               string_list_appendf(&opts->msgs_to_free, msg, cmd, cmd)->string;
+               argv_array_pushf(&opts->msgs_to_free, msg, cmd, cmd);
 
        if (!strcmp(cmd, "checkout"))
                msg = advice_commit_before_merge
@@ -164,7 +161,7 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
                          "Please move or remove them before you %s.")
                      : _("The following untracked working tree files would be 
overwritten by %s:\n%%s");
        msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] =
-               string_list_appendf(&opts->msgs_to_free, msg, cmd, cmd)->string;
+               argv_array_pushf(&opts->msgs_to_free, msg, cmd, cmd);
 
        /*
         * Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we
@@ -189,8 +186,7 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
 
 void clear_unpack_trees_porcelain(struct unpack_trees_options *opts)
 {
-       string_list_clear(&opts->msgs_to_free, 0);
-       memset(opts->msgs, 0, sizeof(opts->msgs));
+       argv_array_clear(&opts->msgs_to_free);
 }
 
 static int do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
diff --git a/unpack-trees.h b/unpack-trees.h
index 5a84123a40..c2b434c606 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -2,7 +2,7 @@
 #define UNPACK_TREES_H
 
 #include "tree-walk.h"
-#include "string-list.h"
+#include "argv-array.h"
 
 #define MAX_UNPACK_TREES 8
 
@@ -62,7 +62,7 @@ struct unpack_trees_options {
        struct pathspec *pathspec;
        merge_fn_t fn;
        const char *msgs[NB_UNPACK_TREES_ERROR_TYPES];
-       struct string_list msgs_to_free;
+       struct argv_array msgs_to_free;
        /*
         * Store error messages in an array, each case
         * corresponding to a error message type
-- 
2.17.0-582-gccdcbd54c4

Reply via email to