From: Junio C Hamano Date: Tue, 22 Aug 2017 17:29:13 +0000 (-0700) Subject: Merge branch 'rs/fsck-obj-leakfix' X-Git-Tag: v2.15.0-rc0~162 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/2d68161a230a6a713ac0f4ad68c51cc856e83338?hp=-c Merge branch 'rs/fsck-obj-leakfix' Memory leak in an error codepath has been plugged. * rs/fsck-obj-leakfix: fsck: free buffers on error in fsck_obj() --- 2d68161a230a6a713ac0f4ad68c51cc856e83338 diff --combined builtin/fsck.c index a92f448186,ab0de7b9d3..b0964a8d34 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@@ -1,6 -1,5 +1,6 @@@ #include "builtin.h" #include "cache.h" +#include "config.h" #include "commit.h" #include "tree.h" #include "blob.h" @@@ -19,8 -18,6 +19,8 @@@ #define REACHABLE 0x0001 #define SEEN 0x0002 #define HAS_OBJ 0x0004 +/* This flag is set if something points to this object. */ +#define USED 0x0008 static int show_root; static int show_tags; @@@ -170,7 -167,18 +170,7 @@@ static void mark_object_reachable(struc static int traverse_one_object(struct object *obj) { - int result; - struct tree *tree = NULL; - - if (obj->type == OBJ_TREE) { - tree = (struct tree *)obj; - if (parse_tree(tree) < 0) - return 1; /* error already displayed */ - } - result = fsck_walk(obj, obj, &fsck_walk_options); - if (tree) - free_tree_buffer(tree); - return result; + return fsck_walk(obj, obj, &fsck_walk_options); } static int traverse_reachable(void) @@@ -197,7 -205,7 +197,7 @@@ static int mark_used(struct object *obj { if (!obj) return 1; - obj->used = 1; + obj->flags |= USED; return 0; } @@@ -246,7 -254,7 +246,7 @@@ static void check_unreachable_object(st } /* - * "!used" means that nothing at all points to it, including + * "!USED" means that nothing at all points to it, including * other unreachable objects. In other words, it's the "tip" * of some set of unreachable objects, usually a commit that * got dropped. @@@ -257,7 -265,7 +257,7 @@@ * deleted a branch by mistake, this is a prime candidate to * start looking at, for example. */ - if (!obj->used) { + if (!(obj->flags & USED)) { if (show_dangling) printf("dangling %s %s\n", printable_type(obj), describe_object(obj)); @@@ -272,7 -280,8 +272,7 @@@ free(filename); return; } - if (!(f = fopen(filename, "w"))) - die_errno("Could not open '%s'", filename); + f = xfopen(filename, "w"); if (obj->type == OBJ_BLOB) { if (stream_blob_to_fd(fileno(f), &obj->oid, NULL, 1)) die_errno("Could not write '%s'", filename); @@@ -326,6 -335,8 +326,8 @@@ static void check_connectivity(void static int fsck_obj(struct object *obj) { + int err; + if (obj->flags & SEEN) return 0; obj->flags |= SEEN; @@@ -336,20 -347,13 +338,13 @@@ if (fsck_walk(obj, NULL, &fsck_obj_options)) objerror(obj, "broken links"); - if (fsck_object(obj, NULL, 0, &fsck_obj_options)) - return -1; - - if (obj->type == OBJ_TREE) { - struct tree *item = (struct tree *) obj; - - free_tree_buffer(item); - } + err = fsck_object(obj, NULL, 0, &fsck_obj_options); + if (err) + goto out; if (obj->type == OBJ_COMMIT) { struct commit *commit = (struct commit *) obj; - free_commit_buffer(commit); - if (!commit->parents && show_root) printf("root %s\n", describe_object(&commit->object)); } @@@ -365,10 -369,15 +360,15 @@@ } } - return 0; + out: + if (obj->type == OBJ_TREE) + free_tree_buffer((struct tree *)obj); + if (obj->type == OBJ_COMMIT) + free_commit_buffer((struct commit *)obj); + return err; } -static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type, +static int fsck_obj_buffer(const struct object_id *oid, enum object_type type, unsigned long size, void *buffer, int *eaten) { /* @@@ -376,51 -385,50 +376,51 @@@ * verify_packfile(), data_valid variable for details. */ struct object *obj; - obj = parse_object_buffer(sha1, type, size, buffer, eaten); + obj = parse_object_buffer(oid, type, size, buffer, eaten); if (!obj) { errors_found |= ERROR_OBJECT; - return error("%s: object corrupt or missing", sha1_to_hex(sha1)); + return error("%s: object corrupt or missing", oid_to_hex(oid)); } - obj->flags = HAS_OBJ; + obj->flags &= ~(REACHABLE | SEEN); + obj->flags |= HAS_OBJ; return fsck_obj(obj); } static int default_refs; -static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1, - unsigned long timestamp) +static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid, + timestamp_t timestamp) { struct object *obj; - if (!is_null_sha1(sha1)) { - obj = lookup_object(sha1); + if (!is_null_oid(oid)) { + obj = lookup_object(oid->hash); if (obj && (obj->flags & HAS_OBJ)) { if (timestamp && name_objects) add_decoration(fsck_walk_options.object_names, obj, - xstrfmt("%s@{%ld}", refname, timestamp)); - obj->used = 1; + xstrfmt("%s@{%"PRItime"}", refname, timestamp)); + obj->flags |= USED; mark_object_reachable(obj); } else { - error("%s: invalid reflog entry %s", refname, sha1_to_hex(sha1)); + error("%s: invalid reflog entry %s", refname, oid_to_hex(oid)); errors_found |= ERROR_REACHABLE; } } } -static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1, - const char *email, unsigned long timestamp, int tz, +static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid, + const char *email, timestamp_t timestamp, int tz, const char *message, void *cb_data) { const char *refname = cb_data; if (verbose) fprintf(stderr, "Checking reflog %s->%s\n", - sha1_to_hex(osha1), sha1_to_hex(nsha1)); + oid_to_hex(ooid), oid_to_hex(noid)); - fsck_handle_reflog_sha1(refname, osha1, 0); - fsck_handle_reflog_sha1(refname, nsha1, timestamp); + fsck_handle_reflog_oid(refname, ooid, 0); + fsck_handle_reflog_oid(refname, noid, timestamp); return 0; } @@@ -436,7 -444,7 +436,7 @@@ static int fsck_handle_ref(const char * { struct object *obj; - obj = parse_object(oid->hash); + obj = parse_object(oid); if (!obj) { error("%s: invalid sha1 pointer %s", refname, oid_to_hex(oid)); errors_found |= ERROR_REACHABLE; @@@ -448,7 -456,7 +448,7 @@@ errors_found |= ERROR_REFS; } default_refs++; - obj->used = 1; + obj->flags |= USED; if (name_objects) add_decoration(fsck_walk_options.object_names, obj, xstrdup(refname)); @@@ -483,7 -491,7 +483,7 @@@ static void get_default_heads(void } } -static struct object *parse_loose_object(const unsigned char *sha1, +static struct object *parse_loose_object(const struct object_id *oid, const char *path) { struct object *obj; @@@ -492,32 -500,31 +492,32 @@@ unsigned long size; int eaten; - if (read_loose_object(path, sha1, &type, &size, &contents) < 0) + if (read_loose_object(path, oid->hash, &type, &size, &contents) < 0) return NULL; if (!contents && type != OBJ_BLOB) die("BUG: read_loose_object streamed a non-blob"); - obj = parse_object_buffer(sha1, type, size, contents, &eaten); + obj = parse_object_buffer(oid, type, size, contents, &eaten); if (!eaten) free(contents); return obj; } -static int fsck_loose(const unsigned char *sha1, const char *path, void *data) +static int fsck_loose(const struct object_id *oid, const char *path, void *data) { - struct object *obj = parse_loose_object(sha1, path); + struct object *obj = parse_loose_object(oid, path); if (!obj) { errors_found |= ERROR_OBJECT; error("%s: object corrupt or missing: %s", - sha1_to_hex(sha1), path); + oid_to_hex(oid), path); return 0; /* keep checking other objects */ } - obj->flags = HAS_OBJ; + obj->flags &= ~(REACHABLE | SEEN); + obj->flags |= HAS_OBJ; if (fsck_obj(obj)) errors_found |= ERROR_OBJECT; return 0; @@@ -530,7 -537,7 +530,7 @@@ static int fsck_cruft(const char *basen return 0; } -static int fsck_subdir(int nr, const char *path, void *progress) +static int fsck_subdir(unsigned int nr, const char *path, void *progress) { display_progress(progress, nr + 1); return 0; @@@ -592,14 -599,14 +592,14 @@@ static int fsck_cache_tree(struct cache fprintf(stderr, "Checking cache tree\n"); if (0 <= it->entry_count) { - struct object *obj = parse_object(it->sha1); + struct object *obj = parse_object(&it->oid); if (!obj) { error("%s: invalid sha1 pointer in cache-tree", - sha1_to_hex(it->sha1)); + oid_to_hex(&it->oid)); errors_found |= ERROR_REFS; return 1; } - obj->used = 1; + obj->flags |= USED; if (name_objects) add_decoration(fsck_walk_options.object_names, obj, xstrdup(":")); @@@ -612,26 -619,26 +612,26 @@@ return err; } -static void mark_object_for_connectivity(const unsigned char *sha1) +static void mark_object_for_connectivity(const struct object_id *oid) { - struct object *obj = lookup_unknown_object(sha1); + struct object *obj = lookup_unknown_object(oid->hash); obj->flags |= HAS_OBJ; } -static int mark_loose_for_connectivity(const unsigned char *sha1, +static int mark_loose_for_connectivity(const struct object_id *oid, const char *path, void *data) { - mark_object_for_connectivity(sha1); + mark_object_for_connectivity(oid); return 0; } -static int mark_packed_for_connectivity(const unsigned char *sha1, +static int mark_packed_for_connectivity(const struct object_id *oid, struct packed_git *pack, uint32_t pos, void *data) { - mark_object_for_connectivity(sha1); + mark_object_for_connectivity(oid); return 0; } @@@ -660,7 -667,7 +660,7 @@@ static struct option fsck_opts[] = int cmd_fsck(int argc, const char **argv, const char *prefix) { - int i, heads; + int i; struct alternate_object_database *alt; errors_found = 0; @@@ -728,23 -735,25 +728,23 @@@ } } - heads = 0; for (i = 0; i < argc; i++) { const char *arg = argv[i]; - unsigned char sha1[20]; - if (!get_sha1(arg, sha1)) { - struct object *obj = lookup_object(sha1); + struct object_id oid; + if (!get_oid(arg, &oid)) { + struct object *obj = lookup_object(oid.hash); if (!obj || !(obj->flags & HAS_OBJ)) { - error("%s: object missing", sha1_to_hex(sha1)); + error("%s: object missing", oid_to_hex(&oid)); errors_found |= ERROR_OBJECT; continue; } - obj->used = 1; + obj->flags |= USED; if (name_objects) add_decoration(fsck_walk_options.object_names, obj, xstrdup(arg)); mark_object_reachable(obj); - heads++; continue; } error("invalid parameter: expected sha1, got '%s'", arg); @@@ -762,7 -771,6 +762,7 @@@ } if (keep_cache_objects) { + verify_index_checksum = 1; read_cache(); for (i = 0; i < active_nr; i++) { unsigned int mode; @@@ -772,11 -780,11 +772,11 @@@ mode = active_cache[i]->ce_mode; if (S_ISGITLINK(mode)) continue; - blob = lookup_blob(active_cache[i]->oid.hash); + blob = lookup_blob(&active_cache[i]->oid); if (!blob) continue; obj = &blob->object; - obj->used = 1; + obj->flags |= USED; if (name_objects) add_decoration(fsck_walk_options.object_names, obj,