mirror of
https://github.com/git/git
synced 2024-08-27 19:49:21 +00:00
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>
This commit is contained in:
parent
edca415256
commit
6e454b9a31
|
@ -16,6 +16,7 @@
|
||||||
|
|
||||||
#define REACHABLE 0x0001
|
#define REACHABLE 0x0001
|
||||||
#define SEEN 0x0002
|
#define SEEN 0x0002
|
||||||
|
#define HAS_OBJ 0x0004
|
||||||
|
|
||||||
static int show_root;
|
static int show_root;
|
||||||
static int show_tags;
|
static int show_tags;
|
||||||
|
@ -101,7 +102,7 @@ static int mark_object(struct object *obj, int type, void *data)
|
||||||
if (obj->flags & REACHABLE)
|
if (obj->flags & REACHABLE)
|
||||||
return 0;
|
return 0;
|
||||||
obj->flags |= REACHABLE;
|
obj->flags |= REACHABLE;
|
||||||
if (!obj->parsed) {
|
if (!(obj->flags & HAS_OBJ)) {
|
||||||
if (parent && !has_sha1_file(obj->sha1)) {
|
if (parent && !has_sha1_file(obj->sha1)) {
|
||||||
printf("broken link from %7s %s\n",
|
printf("broken link from %7s %s\n",
|
||||||
typename(parent->type), sha1_to_hex(parent->sha1));
|
typename(parent->type), sha1_to_hex(parent->sha1));
|
||||||
|
@ -127,16 +128,13 @@ static int traverse_one_object(struct object *obj)
|
||||||
struct tree *tree = NULL;
|
struct tree *tree = NULL;
|
||||||
|
|
||||||
if (obj->type == OBJ_TREE) {
|
if (obj->type == OBJ_TREE) {
|
||||||
obj->parsed = 0;
|
|
||||||
tree = (struct tree *)obj;
|
tree = (struct tree *)obj;
|
||||||
if (parse_tree(tree) < 0)
|
if (parse_tree(tree) < 0)
|
||||||
return 1; /* error already displayed */
|
return 1; /* error already displayed */
|
||||||
}
|
}
|
||||||
result = fsck_walk(obj, mark_object, obj);
|
result = fsck_walk(obj, mark_object, obj);
|
||||||
if (tree) {
|
if (tree)
|
||||||
free(tree->buffer);
|
free_tree_buffer(tree);
|
||||||
tree->buffer = NULL;
|
|
||||||
}
|
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -178,7 +176,7 @@ static void check_reachable_object(struct object *obj)
|
||||||
* except if it was in a pack-file and we didn't
|
* except if it was in a pack-file and we didn't
|
||||||
* do a full fsck
|
* do a full fsck
|
||||||
*/
|
*/
|
||||||
if (!obj->parsed) {
|
if (!(obj->flags & HAS_OBJ)) {
|
||||||
if (has_sha1_pack(obj->sha1))
|
if (has_sha1_pack(obj->sha1))
|
||||||
return; /* it is in pack - forget about it */
|
return; /* it is in pack - forget about it */
|
||||||
printf("missing %s %s\n", typename(obj->type), sha1_to_hex(obj->sha1));
|
printf("missing %s %s\n", typename(obj->type), sha1_to_hex(obj->sha1));
|
||||||
|
@ -306,8 +304,7 @@ static int fsck_obj(struct object *obj)
|
||||||
if (obj->type == OBJ_TREE) {
|
if (obj->type == OBJ_TREE) {
|
||||||
struct tree *item = (struct tree *) obj;
|
struct tree *item = (struct tree *) obj;
|
||||||
|
|
||||||
free(item->buffer);
|
free_tree_buffer(item);
|
||||||
item->buffer = NULL;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (obj->type == OBJ_COMMIT) {
|
if (obj->type == OBJ_COMMIT) {
|
||||||
|
@ -340,6 +337,7 @@ static int fsck_sha1(const unsigned char *sha1)
|
||||||
return error("%s: object corrupt or missing",
|
return error("%s: object corrupt or missing",
|
||||||
sha1_to_hex(sha1));
|
sha1_to_hex(sha1));
|
||||||
}
|
}
|
||||||
|
obj->flags |= HAS_OBJ;
|
||||||
return fsck_obj(obj);
|
return fsck_obj(obj);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -352,6 +350,7 @@ static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,
|
||||||
errors_found |= ERROR_OBJECT;
|
errors_found |= ERROR_OBJECT;
|
||||||
return error("%s: object corrupt or missing", sha1_to_hex(sha1));
|
return error("%s: object corrupt or missing", sha1_to_hex(sha1));
|
||||||
}
|
}
|
||||||
|
obj->flags = HAS_OBJ;
|
||||||
return fsck_obj(obj);
|
return fsck_obj(obj);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -765,6 +765,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
|
||||||
if (obj->type == OBJ_TREE) {
|
if (obj->type == OBJ_TREE) {
|
||||||
struct tree *item = (struct tree *) obj;
|
struct tree *item = (struct tree *) obj;
|
||||||
item->buffer = NULL;
|
item->buffer = NULL;
|
||||||
|
obj->parsed = 0;
|
||||||
}
|
}
|
||||||
if (obj->type == OBJ_COMMIT) {
|
if (obj->type == OBJ_COMMIT) {
|
||||||
struct commit *commit = (struct commit *) obj;
|
struct commit *commit = (struct commit *) obj;
|
||||||
|
|
|
@ -94,8 +94,7 @@ static int tree_is_complete(const unsigned char *sha1)
|
||||||
complete = 0;
|
complete = 0;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
free(tree->buffer);
|
free_tree_buffer(tree);
|
||||||
tree->buffer = NULL;
|
|
||||||
|
|
||||||
if (complete)
|
if (complete)
|
||||||
tree->object.flags |= SEEN;
|
tree->object.flags |= SEEN;
|
||||||
|
|
|
@ -1330,8 +1330,7 @@ static struct object_list **process_tree(struct tree *tree,
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
free(tree->buffer);
|
free_tree_buffer(tree);
|
||||||
tree->buffer = NULL;
|
|
||||||
return p;
|
return p;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -123,8 +123,7 @@ static void process_tree(struct rev_info *revs,
|
||||||
cb_data);
|
cb_data);
|
||||||
}
|
}
|
||||||
strbuf_setlen(base, baselen);
|
strbuf_setlen(base, baselen);
|
||||||
free(tree->buffer);
|
free_tree_buffer(tree);
|
||||||
tree->buffer = NULL;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static void mark_edge_parents_uninteresting(struct commit *commit,
|
static void mark_edge_parents_uninteresting(struct commit *commit,
|
||||||
|
|
|
@ -80,8 +80,7 @@ static void process_tree(struct tree *tree,
|
||||||
else
|
else
|
||||||
process_blob(lookup_blob(entry.sha1), p, &me, entry.path, cp);
|
process_blob(lookup_blob(entry.sha1), p, &me, entry.path, cp);
|
||||||
}
|
}
|
||||||
free(tree->buffer);
|
free_tree_buffer(tree);
|
||||||
tree->buffer = NULL;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static void process_tag(struct tag *tag, struct object_array *p,
|
static void process_tag(struct tag *tag, struct object_array *p,
|
||||||
|
|
|
@ -134,8 +134,7 @@ void mark_tree_uninteresting(struct tree *tree)
|
||||||
* We don't care about the tree any more
|
* We don't care about the tree any more
|
||||||
* after it has been marked uninteresting.
|
* after it has been marked uninteresting.
|
||||||
*/
|
*/
|
||||||
free(tree->buffer);
|
free_tree_buffer(tree);
|
||||||
tree->buffer = NULL;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
void mark_parents_uninteresting(struct commit *commit)
|
void mark_parents_uninteresting(struct commit *commit)
|
||||||
|
|
8
tree.c
8
tree.c
|
@ -225,6 +225,14 @@ int parse_tree(struct tree *item)
|
||||||
return parse_tree_buffer(item, buffer, size);
|
return parse_tree_buffer(item, buffer, size);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void free_tree_buffer(struct tree *tree)
|
||||||
|
{
|
||||||
|
free(tree->buffer);
|
||||||
|
tree->buffer = NULL;
|
||||||
|
tree->size = 0;
|
||||||
|
tree->object.parsed = 0;
|
||||||
|
}
|
||||||
|
|
||||||
struct tree *parse_tree_indirect(const unsigned char *sha1)
|
struct tree *parse_tree_indirect(const unsigned char *sha1)
|
||||||
{
|
{
|
||||||
struct object *obj = parse_object(sha1);
|
struct object *obj = parse_object(sha1);
|
||||||
|
|
1
tree.h
1
tree.h
|
@ -16,6 +16,7 @@ struct tree *lookup_tree(const unsigned char *sha1);
|
||||||
int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size);
|
int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size);
|
||||||
|
|
||||||
int parse_tree(struct tree *tree);
|
int parse_tree(struct tree *tree);
|
||||||
|
void free_tree_buffer(struct tree *tree);
|
||||||
|
|
||||||
/* Parses and returns the tree in the given ent, chasing tags and commits. */
|
/* Parses and returns the tree in the given ent, chasing tags and commits. */
|
||||||
struct tree *parse_tree_indirect(const unsigned char *sha1);
|
struct tree *parse_tree_indirect(const unsigned char *sha1);
|
||||||
|
|
5
walker.c
5
walker.c
|
@ -56,10 +56,7 @@ static int process_tree(struct walker *walker, struct tree *tree)
|
||||||
if (!obj || process(walker, obj))
|
if (!obj || process(walker, obj))
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
free(tree->buffer);
|
free_tree_buffer(tree);
|
||||||
tree->buffer = NULL;
|
|
||||||
tree->size = 0;
|
|
||||||
tree->object.parsed = 0;
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue