@matheusbsbs / matheus-bagliano / commits / 1c41d2805e

unpack_trees_options: free messages when done

The strings allocated in `setup_unpack_trees_porcelain()` are never freed. Provide a function `clear_unpack_trees_porcelain()` to do so and call it where we use `setup_unpack_trees_porcelain()`. The only non-trivial user is `unpack_trees_start()`, where we should place the new call in `unpack_trees_finish()`. We keep the string pointers in an array, mixing pointers to static memory and memory that we allocate on the heap. We also keep several copies of the individual pointers. So we need to make sure that we do not free what we must not free and that we do not double-free. Let a separate argv_array take ownership of all the strings we create so that we can easily free them. Zero the whole array of string pointers to make sure that we do not leave any dangling pointers. Note that we only take responsibility for the memory allocated in `setup_unpack_trees_porcelain()` and not any other members of the `struct unpack_trees_options`. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed May 21, 2018 at 16:54 UTC 1c41d2805e42d77d943fd3d79ebf5136f74c9ba3

5 files changed +26 -4

builtin/checkout.c

@@ -526,6 +526,7 @@ static int merge_working_tree(const struct checkout_opts *opts,

526 init_tree_desc(&trees[1], tree->buffer, tree->size);

527

528 ret = unpack_trees(2, trees, &topts);

529 + clear_unpack_trees_porcelain(&topts);

530 if (ret == -1) {

531 /*

532 * Unpack couldn't do a trivial merge; either

merge-recursive.c

@@ -382,6 +382,7 @@ static int unpack_trees_start(struct merge_options *o,

382 static void unpack_trees_finish(struct merge_options *o)

383 {

384 discard_index(&o->orig_index);

385 + clear_unpack_trees_porcelain(&o->unpack_opts);

386 }

387

388 struct tree *write_tree_from_memory(struct merge_options *o)

merge.c

@@ -130,8 +130,11 @@ int checkout_fast_forward(const struct object_id *head,

130

131 if (unpack_trees(nr_trees, t, &opts)) {

132 rollback_lock_file(&lock_file);

133 + clear_unpack_trees_porcelain(&opts);

134 return -1;

135 }

136 + clear_unpack_trees_porcelain(&opts);

137 +

138 if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))

139 return error(_("unable to write new index file"));

140 return 0;

unpack-trees.c

+14 -3

@@ -1,5 +1,6 @@

1 #define NO_THE_INDEX_COMPATIBILITY_MACROS

2 #include "cache.h"

3 +#include "argv-array.h"

4 #include "repository.h"

5 #include "config.h"

6 #include "dir.h"

@@ -103,6 +104,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,

104 const char **msgs = opts->msgs;

105 const char *msg;

106

107 + argv_array_init(&opts->msgs_to_free);

108 +

109 if (!strcmp(cmd, "checkout"))

110 msg = advice_commit_before_merge

111 ? _("Your local changes to the following files would be overwritten by checkout:\n%%s"

@@ -119,7 +122,7 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,

122 "Please commit your changes or stash them before you %s.")

123 : _("Your local changes to the following files would be overwritten by %s:\n%%s");

124 msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] =

122 - xstrfmt(msg, cmd, cmd);

125 + argv_array_pushf(&opts->msgs_to_free, msg, cmd, cmd);

126

127 msgs[ERROR_NOT_UPTODATE_DIR] =

128 _("Updating the following directories would lose untracked files in them:\n%s");

@@ -139,7 +142,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,

142 ? _("The following untracked working tree files would be removed by %s:\n%%s"

143 "Please move or remove them before you %s.")

144 : _("The following untracked working tree files would be removed by %s:\n%%s");

142 - msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = xstrfmt(msg, cmd, cmd);

145 + msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] =

146 + argv_array_pushf(&opts->msgs_to_free, msg, cmd, cmd);

147

148 if (!strcmp(cmd, "checkout"))

149 msg = advice_commit_before_merge

@@ -156,7 +160,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,

160 ? _("The following untracked working tree files would be overwritten by %s:\n%%s"

161 "Please move or remove them before you %s.")

162 : _("The following untracked working tree files would be overwritten by %s:\n%%s");

159 - msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = xstrfmt(msg, cmd, cmd);

163 + msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] =

164 + argv_array_pushf(&opts->msgs_to_free, msg, cmd, cmd);

165

166 /*

167 * Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we

@@ -179,6 +184,12 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,

184 opts->unpack_rejects[i].strdup_strings = 1;

185 }

186

187 +void clear_unpack_trees_porcelain(struct unpack_trees_options *opts)

188 +{

189 + argv_array_clear(&opts->msgs_to_free);

190 + memset(opts->msgs, 0, sizeof(opts->msgs));

191 +}

192 +

193 static int do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce,

194 unsigned int set, unsigned int clear)

195 {

unpack-trees.h

+7 -1

@@ -2,7 +2,7 @@

2 #define UNPACK_TREES_H

4 #include "tree-walk.h"

5 -#include "string-list.h"

5 +#include "argv-array.h"

7 #define MAX_UNPACK_TREES 8

@@ -33,6 +33,11 @@ enum unpack_trees_error_types {

33 void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,

34 const char *cmd);

36 +/*

37 + * Frees resources allocated by setup_unpack_trees_porcelain().

38 + */

39 +void clear_unpack_trees_porcelain(struct unpack_trees_options *opts);

40 +

41 struct unpack_trees_options {

42 unsigned int reset,

43 merge,

@@ -57,6 +62,7 @@ struct unpack_trees_options {

62 struct pathspec *pathspec;

63 merge_fn_t fn;

64 const char *msgs[NB_UNPACK_TREES_ERROR_TYPES];

65 + struct argv_array msgs_to_free;

66 /*

67 * Store error messages in an array, each case

68 * corresponding to a error message type