@matheusbsbs / matheus-bagliano / commits / 6e454b9a31

clear parsed flag when we free tree buffers

Many code paths will free a tree object's buffer and set it to NULL after finishing with it in order to keep memory usage down during a traversal. However, out of 8 sites that do this, only one actually unsets the "parsed" flag back. Those sites that don't are setting a trap for later users of the tree object; even after calling parse_tree, the buffer will remain NULL, causing potential segfaults. It is not known whether this is triggerable in the current code. Most commands do not do an in-memory traversal followed by actually using the objects again. However, it does not hurt to be safe for future callers. In most cases, we can abstract this out to a "free_tree_buffer" helper. However, there are two exceptions: 1. The fsck code relies on the parsed flag to know that we were able to parse the object at one point. We can switch this to using a flag in the "flags" field. 2. The index-pack code sets the buffer to NULL but does not free it (it is freed by a caller). We should still unset the parsed flag here, but we cannot use our helper, as we do not want to free the buffer. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 5, 2013 at 18:37 UTC 6e454b9a31840102807f1eee527ee717bf134102

10 files changed +24 -23

builtin/fsck.c

+8 -9

@@ -16,6 +16,7 @@

17 #define REACHABLE 0x0001

18 #define SEEN 0x0002

19 +#define HAS_OBJ 0x0004

21 static int show_root;

22 static int show_tags;

@@ -101,7 +102,7 @@ static int mark_object(struct object *obj, int type, void *data)

102 if (obj->flags & REACHABLE)

103 return 0;

104 obj->flags |= REACHABLE;

104 - if (!obj->parsed) {

105 + if (!(obj->flags & HAS_OBJ)) {

106 if (parent && !has_sha1_file(obj->sha1)) {

107 printf("broken link from %7s %s\n",

108 typename(parent->type), sha1_to_hex(parent->sha1));

@@ -127,16 +128,13 @@ static int traverse_one_object(struct object *obj)

128 struct tree *tree = NULL;

129

130 if (obj->type == OBJ_TREE) {

130 - obj->parsed = 0;

131 tree = (struct tree *)obj;

132 if (parse_tree(tree) < 0)

133 return 1; /* error already displayed */

134 }

135 result = fsck_walk(obj, mark_object, obj);

136 - if (tree) {

137 - free(tree->buffer);

138 - tree->buffer = NULL;

139 - }

136 + if (tree)

137 + free_tree_buffer(tree);

138 return result;

139 }

140

@@ -178,7 +176,7 @@ static void check_reachable_object(struct object *obj)

176 * except if it was in a pack-file and we didn't

177 * do a full fsck

178 */

181 - if (!obj->parsed) {

179 + if (!(obj->flags & HAS_OBJ)) {

180 if (has_sha1_pack(obj->sha1))

181 return; /* it is in pack - forget about it */

182 printf("missing %s %s\n", typename(obj->type), sha1_to_hex(obj->sha1));

@@ -306,8 +304,7 @@ static int fsck_obj(struct object *obj)

304 if (obj->type == OBJ_TREE) {

305 struct tree *item = (struct tree *) obj;

306

309 - free(item->buffer);

310 - item->buffer = NULL;

307 + free_tree_buffer(item);

308 }

309

310 if (obj->type == OBJ_COMMIT) {

@@ -340,6 +337,7 @@ static int fsck_sha1(const unsigned char *sha1)

337 return error("%s: object corrupt or missing",

338 sha1_to_hex(sha1));

339 }

340 + obj->flags |= HAS_OBJ;

341 return fsck_obj(obj);

342 }

343

@@ -352,6 +350,7 @@ static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,

350 errors_found |= ERROR_OBJECT;

351 return error("%s: object corrupt or missing", sha1_to_hex(sha1));

352 }

353 + obj->flags = HAS_OBJ;

354 return fsck_obj(obj);

355 }

356

builtin/index-pack.c

@@ -765,6 +765,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,

765 if (obj->type == OBJ_TREE) {

766 struct tree *item = (struct tree *) obj;

767 item->buffer = NULL;

768 + obj->parsed = 0;

769 }

770 if (obj->type == OBJ_COMMIT) {

771 struct commit *commit = (struct commit *) obj;

builtin/reflog.c

+1 -2

@@ -94,8 +94,7 @@ static int tree_is_complete(const unsigned char *sha1)

94 complete = 0;

95 }

96 }

97 - free(tree->buffer);

98 - tree->buffer = NULL;

97 + free_tree_buffer(tree);

99 if (complete)

100 tree->object.flags |= SEEN;

http-push.c

+1 -2

@@ -1330,8 +1330,7 @@ static struct object_list **process_tree(struct tree *tree,

1330 break;

1331 }

1332

1333 - free(tree->buffer);

1334 - tree->buffer = NULL;

1333 + free_tree_buffer(tree);

1334 return p;

1335 }

1336

list-objects.c

+1 -2

@@ -123,8 +123,7 @@ static void process_tree(struct rev_info *revs,

123 cb_data);

124 }

125 strbuf_setlen(base, baselen);

126 - free(tree->buffer);

127 - tree->buffer = NULL;

126 + free_tree_buffer(tree);

127 }

128

129 static void mark_edge_parents_uninteresting(struct commit *commit,

reachable.c

+1 -2

@@ -80,8 +80,7 @@ static void process_tree(struct tree *tree,

80 else

81 process_blob(lookup_blob(entry.sha1), p, &me, entry.path, cp);

82 }

83 - free(tree->buffer);

84 - tree->buffer = NULL;

83 + free_tree_buffer(tree);

84 }

86 static void process_tag(struct tag *tag, struct object_array *p,

revision.c

+1 -2

@@ -134,8 +134,7 @@ void mark_tree_uninteresting(struct tree *tree)

134 * We don't care about the tree any more

135 * after it has been marked uninteresting.

136 */

137 - free(tree->buffer);

138 - tree->buffer = NULL;

137 + free_tree_buffer(tree);

138 }

139

140 void mark_parents_uninteresting(struct commit *commit)

tree.c

@@ -225,6 +225,14 @@ int parse_tree(struct tree *item)

225 return parse_tree_buffer(item, buffer, size);

226 }

227

228 +void free_tree_buffer(struct tree *tree)

229 +{

230 + free(tree->buffer);

231 + tree->buffer = NULL;

232 + tree->size = 0;

233 + tree->object.parsed = 0;

234 +}

235 +

236 struct tree *parse_tree_indirect(const unsigned char *sha1)

237 {

238 struct object *obj = parse_object(sha1);

tree.h

@@ -16,6 +16,7 @@ struct tree *lookup_tree(const unsigned char *sha1);

16 int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size);

18 int parse_tree(struct tree *tree);

19 +void free_tree_buffer(struct tree *tree);

21 /* Parses and returns the tree in the given ent, chasing tags and commits. */

22 struct tree *parse_tree_indirect(const unsigned char *sha1);

walker.c

+1 -4

@@ -56,10 +56,7 @@ static int process_tree(struct walker *walker, struct tree *tree)

56 if (!obj || process(walker, obj))

57 return -1;

58 }

59 - free(tree->buffer);

60 - tree->buffer = NULL;

61 - tree->size = 0;

62 - tree->object.parsed = 0;

59 + free_tree_buffer(tree);

60 return 0;

61 }