From 25fba78d36be6297bb17aed5f3e21ed850ce3e03 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 12 Jul 2013 02:20:05 -0400 Subject: [PATCH 1/8] cat-file: disable object/refname ambiguity check for batch mode A common use of "cat-file --batch-check" is to feed a list of objects from "rev-list --objects" or a similar command. In this instance, all of our input objects are 40-byte sha1 ids. However, cat-file has always allowed arbitrary revision specifiers, and feeds the result to get_sha1(). Fortunately, get_sha1() recognizes a 40-byte sha1 before doing any hard work trying to look up refs, meaning this scenario should end up spending very little time converting the input into an object sha1. However, since 798c35f (get_sha1: warn about full or short object names that look like refs, 2013-05-29), when we encounter this case, we spend the extra effort to do a refname lookup anyway, just to print a warning. This is further exacerbated by ca91993 (get_packed_ref_cache: reload packed-refs file when it changes, 2013-06-20), which makes individual ref lookup more expensive by requiring a stat() of the packed-refs file for each missing ref. With no patches, this is the time it takes to run: $ git rev-list --objects --all >objects $ time git cat-file --batch-check='%(objectname)' Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 9 +++++++++ cache.h | 1 + environment.c | 1 + sha1_name.c | 14 ++++++++------ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 0e64b4159c..fe5c77f54c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -266,6 +266,15 @@ static int batch_objects(struct batch_options *opt) strbuf_expand(&buf, opt->format, expand_format, &data); data.mark_query = 0; + /* + * We are going to call get_sha1 on a potentially very large number of + * objects. In most large cases, these will be actual object sha1s. The + * cost to double-check that each one is not also a ref (just so we can + * warn) ends up dwarfing the actual cost of the object lookups + * themselves. We can work around it by just turning off the warning. + */ + warn_on_object_refname_ambiguity = 0; + while (strbuf_getline(&buf, stdin, '\n') != EOF) { char *p; int error; diff --git a/cache.h b/cache.h index f2915509a6..dc99366a2e 100644 --- a/cache.h +++ b/cache.h @@ -554,6 +554,7 @@ extern int assume_unchanged; extern int prefer_symlink_refs; extern int log_all_ref_updates; extern int warn_ambiguous_refs; +extern int warn_on_object_refname_ambiguity; extern int shared_repository; extern const char *apply_default_whitespace; extern const char *apply_default_ignorewhitespace; diff --git a/environment.c b/environment.c index e2e75c1660..1a99e47251 100644 --- a/environment.c +++ b/environment.c @@ -22,6 +22,7 @@ int prefer_symlink_refs; int is_bare_repository_cfg = -1; /* unspecified */ int log_all_ref_updates = -1; /* unspecified */ int warn_ambiguous_refs = 1; +int warn_on_object_refname_ambiguity = 1; int repository_format_version; const char *git_commit_encoding; const char *git_log_output_encoding; diff --git a/sha1_name.c b/sha1_name.c index a1e0f804dc..c1957f35c1 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -451,13 +451,15 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) int at, reflog_len; if (len == 40 && !get_sha1_hex(str, sha1)) { - refs_found = dwim_ref(str, len, tmp_sha1, &real_ref); - if (refs_found > 0 && warn_ambiguous_refs) { - warning(warn_msg, len, str); - if (advice_object_name_warning) - fprintf(stderr, "%s\n", _(object_name_msg)); + if (warn_on_object_refname_ambiguity) { + refs_found = dwim_ref(str, len, tmp_sha1, &real_ref); + if (refs_found > 0 && warn_ambiguous_refs) { + warning(warn_msg, len, str); + if (advice_object_name_warning) + fprintf(stderr, "%s\n", _(object_name_msg)); + } + free(real_ref); } - free(real_ref); return 0; } From f2f57e31f671983ef4d6272c082f9fb06daa3429 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 12 Jul 2013 02:21:22 -0400 Subject: [PATCH 2/8] sha1_object_info_extended: rename "status" to "type" The value we get from each low-level object_info function (e.g., loose, packed) is actually the object type (or -1 for error). Let's explicitly call it "type", which will make further refactorings easier to read. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_file.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 6baed676dc..f13dea3355 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2375,7 +2375,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) { struct cached_object *co; struct pack_entry e; - int status, rtype; + int type, rtype; co = find_cached_object(sha1); if (co) { @@ -2389,23 +2389,23 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) if (!find_pack_entry(sha1, &e)) { /* Most likely it's a loose object. */ - status = sha1_loose_object_info(sha1, oi->sizep, oi->disk_sizep); - if (status >= 0) { + type = sha1_loose_object_info(sha1, oi->sizep, oi->disk_sizep); + if (type >= 0) { oi->whence = OI_LOOSE; - return status; + return type; } /* Not a loose object; someone else may have just packed it. */ reprepare_packed_git(); if (!find_pack_entry(sha1, &e)) - return status; + return type; } - status = packed_object_info(e.p, e.offset, oi->sizep, &rtype, - oi->disk_sizep); - if (status < 0) { + type = packed_object_info(e.p, e.offset, oi->sizep, &rtype, + oi->disk_sizep); + if (type < 0) { mark_bad_packed_object(e.p, sha1); - status = sha1_object_info_extended(sha1, oi); + type = sha1_object_info_extended(sha1, oi); } else if (in_delta_base_cache(e.p, e.offset)) { oi->whence = OI_DBCACHED; } else { @@ -2416,7 +2416,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) rtype == OBJ_OFS_DELTA); } - return status; + return type; } int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) From 052fe5eaca92cac810cdb7f89bd8860fc2d5f0e1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 12 Jul 2013 02:30:48 -0400 Subject: [PATCH 3/8] sha1_loose_object_info: make type lookup optional Until recently, the only items to request from sha1_object_info_extended were type and size. This meant that we always had to open a loose object file to determine one or the other. But with the addition of the disk_size query, it's possible that we can fulfill the query without even opening the object file at all. However, since the function interface always returns the type, we have no way of knowing whether the caller cares about it or not. This patch only modified sha1_loose_object_info to make type lookup optional using an out-parameter, similar to the way the size is handled (and the return value is "0" or "-1" for success or error, respectively). There should be no functional change yet, though, as sha1_object_info_extended, the only caller, will always ask for a type. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_file.c | 48 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index f13dea3355..285337819e 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1303,6 +1303,26 @@ static int git_open_noatime(const char *name) } } +static int stat_sha1_file(const unsigned char *sha1, struct stat *st) +{ + char *name = sha1_file_name(sha1); + struct alternate_object_database *alt; + + if (!lstat(name, st)) + return 0; + + prepare_alt_odb(); + errno = ENOENT; + for (alt = alt_odb_list; alt; alt = alt->next) { + name = alt->name; + fill_sha1_path(name, sha1); + if (!lstat(alt->base, st)) + return 0; + } + + return -1; +} + static int open_sha1_file(const unsigned char *sha1) { int fd; @@ -2344,7 +2364,9 @@ struct packed_git *find_sha1_pack(const unsigned char *sha1, } -static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *sizep, +static int sha1_loose_object_info(const unsigned char *sha1, + enum object_type *typep, + unsigned long *sizep, unsigned long *disk_sizep) { int status; @@ -2353,6 +2375,20 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size git_zstream stream; char hdr[32]; + /* + * If we don't care about type or size, then we don't + * need to look inside the object at all. + */ + if (!typep && !sizep) { + if (disk_sizep) { + struct stat st; + if (stat_sha1_file(sha1, &st) < 0) + return -1; + *disk_sizep = st.st_size; + } + return 0; + } + map = map_sha1_file(sha1, &mapsize); if (!map) return error("unable to find %s", sha1_to_hex(sha1)); @@ -2367,7 +2403,9 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size *sizep = size; git_inflate_end(&stream); munmap(map, mapsize); - return status; + if (typep) + *typep = status; + return 0; } /* returns enum object_type or negative */ @@ -2389,8 +2427,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) if (!find_pack_entry(sha1, &e)) { /* Most likely it's a loose object. */ - type = sha1_loose_object_info(sha1, oi->sizep, oi->disk_sizep); - if (type >= 0) { + if (!sha1_loose_object_info(sha1, &type, + oi->sizep, oi->disk_sizep)) { oi->whence = OI_LOOSE; return type; } @@ -2398,7 +2436,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) /* Not a loose object; someone else may have just packed it. */ reprepare_packed_git(); if (!find_pack_entry(sha1, &e)) - return type; + return -1; } type = packed_object_info(e.p, e.offset, oi->sizep, &rtype, From 90191d37ab08f7c6810fa22804dedb23bade02a1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 12 Jul 2013 02:31:57 -0400 Subject: [PATCH 4/8] packed_object_info: hoist delta type resolution to helper To calculate the type of a packed object, we must walk down its delta chain until we hit a true base object with a real type. Most of the code in packed_object_info is for handling this case. Let's hoist it out into a separate helper function, which will make it easier to make the type-lookup optional in the future (and keep our indentation level sane). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_file.c | 123 +++++++++++++++++++++++++++++----------------------- 1 file changed, 68 insertions(+), 55 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 285337819e..8fb10e451c 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1710,9 +1710,75 @@ static int retry_bad_packed_offset(struct packed_git *p, off_t obj_offset) return type; } - #define POI_STACK_PREALLOC 64 +static enum object_type packed_to_object_type(struct packed_git *p, + off_t obj_offset, + enum object_type type, + struct pack_window **w_curs, + off_t curpos) +{ + off_t small_poi_stack[POI_STACK_PREALLOC]; + off_t *poi_stack = small_poi_stack; + int poi_stack_nr = 0, poi_stack_alloc = POI_STACK_PREALLOC; + + while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) { + off_t base_offset; + unsigned long size; + /* Push the object we're going to leave behind */ + if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) { + poi_stack_alloc = alloc_nr(poi_stack_nr); + poi_stack = xmalloc(sizeof(off_t)*poi_stack_alloc); + memcpy(poi_stack, small_poi_stack, sizeof(off_t)*poi_stack_nr); + } else { + ALLOC_GROW(poi_stack, poi_stack_nr+1, poi_stack_alloc); + } + poi_stack[poi_stack_nr++] = obj_offset; + /* If parsing the base offset fails, just unwind */ + base_offset = get_delta_base(p, w_curs, &curpos, type, obj_offset); + if (!base_offset) + goto unwind; + curpos = obj_offset = base_offset; + type = unpack_object_header(p, w_curs, &curpos, &size); + if (type <= OBJ_NONE) { + /* If getting the base itself fails, we first + * retry the base, otherwise unwind */ + type = retry_bad_packed_offset(p, base_offset); + if (type > OBJ_NONE) + goto out; + goto unwind; + } + } + + switch (type) { + case OBJ_BAD: + case OBJ_COMMIT: + case OBJ_TREE: + case OBJ_BLOB: + case OBJ_TAG: + break; + default: + error("unknown object type %i at offset %"PRIuMAX" in %s", + type, (uintmax_t)obj_offset, p->pack_name); + type = OBJ_BAD; + } + +out: + if (poi_stack != small_poi_stack) + free(poi_stack); + return type; + +unwind: + while (poi_stack_nr) { + obj_offset = poi_stack[--poi_stack_nr]; + type = retry_bad_packed_offset(p, obj_offset); + if (type > OBJ_NONE) + goto out; + } + type = OBJ_BAD; + goto out; +} + static int packed_object_info(struct packed_git *p, off_t obj_offset, unsigned long *sizep, int *rtype, unsigned long *disk_sizep) @@ -1721,9 +1787,6 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset, unsigned long size; off_t curpos = obj_offset; enum object_type type; - off_t small_poi_stack[POI_STACK_PREALLOC]; - off_t *poi_stack = small_poi_stack; - int poi_stack_nr = 0, poi_stack_alloc = POI_STACK_PREALLOC; type = unpack_object_header(p, &w_curs, &curpos, &size); @@ -1754,61 +1817,11 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset, *disk_sizep = revidx[1].offset - obj_offset; } - while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) { - off_t base_offset; - /* Push the object we're going to leave behind */ - if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) { - poi_stack_alloc = alloc_nr(poi_stack_nr); - poi_stack = xmalloc(sizeof(off_t)*poi_stack_alloc); - memcpy(poi_stack, small_poi_stack, sizeof(off_t)*poi_stack_nr); - } else { - ALLOC_GROW(poi_stack, poi_stack_nr+1, poi_stack_alloc); - } - poi_stack[poi_stack_nr++] = obj_offset; - /* If parsing the base offset fails, just unwind */ - base_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset); - if (!base_offset) - goto unwind; - curpos = obj_offset = base_offset; - type = unpack_object_header(p, &w_curs, &curpos, &size); - if (type <= OBJ_NONE) { - /* If getting the base itself fails, we first - * retry the base, otherwise unwind */ - type = retry_bad_packed_offset(p, base_offset); - if (type > OBJ_NONE) - goto out; - goto unwind; - } - } - - switch (type) { - case OBJ_BAD: - case OBJ_COMMIT: - case OBJ_TREE: - case OBJ_BLOB: - case OBJ_TAG: - break; - default: - error("unknown object type %i at offset %"PRIuMAX" in %s", - type, (uintmax_t)obj_offset, p->pack_name); - type = OBJ_BAD; - } + type = packed_to_object_type(p, obj_offset, type, &w_curs, curpos); out: - if (poi_stack != small_poi_stack) - free(poi_stack); unuse_pack(&w_curs); return type; - -unwind: - while (poi_stack_nr) { - obj_offset = poi_stack[--poi_stack_nr]; - type = retry_bad_packed_offset(p, obj_offset); - if (type > OBJ_NONE) - goto out; - } - type = OBJ_BAD; - goto out; } static void *unpack_compressed_entry(struct packed_git *p, From 412916ee131cf8b0aaac96313ce8686aebe3a47a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 12 Jul 2013 02:32:25 -0400 Subject: [PATCH 5/8] packed_object_info: make type lookup optional Currently, packed_object_info can save some work by not calculating the size or disk_size of the object if the caller is not interested. However, it always calculates the true object type, whether the caller cares or not, and only optionally returns the easy-to-get "representation type". Let's swap these types. The function will now return the representation type (or OBJ_BAD on failure), and will only optionally fill in the true type. There should be no behavior change yet, as the only caller, sha1_object_info_extended, will always feed it a type pointer. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_file.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 8fb10e451c..fa2809884b 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1780,7 +1780,7 @@ unwind: } static int packed_object_info(struct packed_git *p, off_t obj_offset, - unsigned long *sizep, int *rtype, + enum object_type *typep, unsigned long *sizep, unsigned long *disk_sizep) { struct pack_window *w_curs = NULL; @@ -1788,11 +1788,12 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset, off_t curpos = obj_offset; enum object_type type; + /* + * We always get the representation type, but only convert it to + * a "real" type later if the caller is interested. + */ type = unpack_object_header(p, &w_curs, &curpos, &size); - if (rtype) - *rtype = type; /* representation type */ - if (sizep) { if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) { off_t tmp_pos = curpos; @@ -1817,7 +1818,13 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset, *disk_sizep = revidx[1].offset - obj_offset; } - type = packed_to_object_type(p, obj_offset, type, &w_curs, curpos); + if (typep) { + *typep = packed_to_object_type(p, obj_offset, type, &w_curs, curpos); + if (*typep < 0) { + type = OBJ_BAD; + goto out; + } + } out: unuse_pack(&w_curs); @@ -2452,11 +2459,11 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) return -1; } - type = packed_object_info(e.p, e.offset, oi->sizep, &rtype, - oi->disk_sizep); - if (type < 0) { + rtype = packed_object_info(e.p, e.offset, &type, oi->sizep, + oi->disk_sizep); + if (rtype < 0) { mark_bad_packed_object(e.p, sha1); - type = sha1_object_info_extended(sha1, oi); + return sha1_object_info_extended(sha1, oi); } else if (in_delta_base_cache(e.p, e.offset)) { oi->whence = OI_DBCACHED; } else { From 5b0864070e1e64683e49464c77a72f3c528c8f71 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 12 Jul 2013 02:34:57 -0400 Subject: [PATCH 6/8] sha1_object_info_extended: make type calculation optional Each caller of sha1_object_info_extended sets up an object_info struct to tell the function which elements of the object it wants to get. Until now, getting the type of the object has always been required (and it is returned via the return type rather than a pointer in object_info). This can involve actually opening a loose object file to determine its type, or following delta chains to determine a packed file's base type. These effects produce a measurable slow-down when doing a "cat-file --batch-check" that does not include %(objecttype). This patch adds a "typep" query to struct object_info, so that it can be optionally queried just like size and disk_size. As a result, the return type of the function is no longer the object type, but rather 0/-1 for success/error. As there are only three callers total, we just fix up each caller rather than keep a compatibility wrapper: 1. The simpler sha1_object_info wrapper continues to always ask for and return the type field. 2. The istream_source function wants to know the type, and so always asks for it. 3. The cat-file batch code asks for the type only when %(objecttype) is part of the format string. On linux.git, the best-of-five for running: $ git rev-list --objects --all >objects $ time git cat-file --batch-check='%(objectsize:disk)' on a fully packed repository goes from: real 0m8.680s user 0m8.160s sys 0m0.512s to: real 0m7.205s user 0m6.580s sys 0m0.608s Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 7 ++++--- cache.h | 1 + sha1_file.c | 20 +++++++++++++------- streaming.c | 2 +- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index fe5c77f54c..163ce6c77c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -150,7 +150,9 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, if (!data->mark_query) strbuf_addstr(sb, sha1_to_hex(data->sha1)); } else if (is_atom("objecttype", atom, len)) { - if (!data->mark_query) + if (data->mark_query) + data->info.typep = &data->type; + else strbuf_addstr(sb, typename(data->type)); } else if (is_atom("objectsize", atom, len)) { if (data->mark_query) @@ -229,8 +231,7 @@ static int batch_one_object(const char *obj_name, struct batch_options *opt, return 0; } - data->type = sha1_object_info_extended(data->sha1, &data->info); - if (data->type <= 0) { + if (sha1_object_info_extended(data->sha1, &data->info) < 0) { printf("%s missing\n", obj_name); fflush(stdout); return 0; diff --git a/cache.h b/cache.h index dc99366a2e..6882bbe6c3 100644 --- a/cache.h +++ b/cache.h @@ -1099,6 +1099,7 @@ extern int unpack_object_header(struct packed_git *, struct pack_window **, off_ struct object_info { /* Request */ + enum object_type *typep; unsigned long *sizep; unsigned long *disk_sizep; diff --git a/sha1_file.c b/sha1_file.c index fa2809884b..d391271310 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2433,24 +2433,26 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) { struct cached_object *co; struct pack_entry e; - int type, rtype; + int rtype; co = find_cached_object(sha1); if (co) { + if (oi->typep) + *(oi->typep) = co->type; if (oi->sizep) *(oi->sizep) = co->size; if (oi->disk_sizep) *(oi->disk_sizep) = 0; oi->whence = OI_CACHED; - return co->type; + return 0; } if (!find_pack_entry(sha1, &e)) { /* Most likely it's a loose object. */ - if (!sha1_loose_object_info(sha1, &type, + if (!sha1_loose_object_info(sha1, oi->typep, oi->sizep, oi->disk_sizep)) { oi->whence = OI_LOOSE; - return type; + return 0; } /* Not a loose object; someone else may have just packed it. */ @@ -2459,7 +2461,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) return -1; } - rtype = packed_object_info(e.p, e.offset, &type, oi->sizep, + rtype = packed_object_info(e.p, e.offset, oi->typep, oi->sizep, oi->disk_sizep); if (rtype < 0) { mark_bad_packed_object(e.p, sha1); @@ -2474,15 +2476,19 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) rtype == OBJ_OFS_DELTA); } - return type; + return 0; } int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) { + enum object_type type; struct object_info oi = {0}; + oi.typep = &type; oi.sizep = sizep; - return sha1_object_info_extended(sha1, &oi); + if (sha1_object_info_extended(sha1, &oi) < 0) + return -1; + return type; } static void *read_packed_sha1(const unsigned char *sha1, diff --git a/streaming.c b/streaming.c index cac282f06b..870657ab56 100644 --- a/streaming.c +++ b/streaming.c @@ -111,11 +111,11 @@ static enum input_source istream_source(const unsigned char *sha1, unsigned long size; int status; + oi->typep = type; oi->sizep = &size; status = sha1_object_info_extended(sha1, oi); if (status < 0) return stream_error; - *type = status; switch (oi->whence) { case OI_LOOSE: From 23c339c0f262aad2515e48c33ec8f35d476d2ddc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 12 Jul 2013 02:37:53 -0400 Subject: [PATCH 7/8] sha1_object_info_extended: pass object_info to helpers We take in a "struct object_info" which contains pointers to storage for items the caller cares about. But then rather than pass the whole object to the low-level loose/packed helper functions, we pass the individual pointers. Let's pass the whole struct instead, which will make adding more items later easier. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_file.c | 49 ++++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index d391271310..2500b94478 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1780,8 +1780,7 @@ unwind: } static int packed_object_info(struct packed_git *p, off_t obj_offset, - enum object_type *typep, unsigned long *sizep, - unsigned long *disk_sizep) + struct object_info *oi) { struct pack_window *w_curs = NULL; unsigned long size; @@ -1794,7 +1793,7 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset, */ type = unpack_object_header(p, &w_curs, &curpos, &size); - if (sizep) { + if (oi->sizep) { if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) { off_t tmp_pos = curpos; off_t base_offset = get_delta_base(p, &w_curs, &tmp_pos, @@ -1803,24 +1802,24 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset, type = OBJ_BAD; goto out; } - *sizep = get_size_from_delta(p, &w_curs, tmp_pos); - if (*sizep == 0) { + *oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos); + if (*oi->sizep == 0) { type = OBJ_BAD; goto out; } } else { - *sizep = size; + *oi->sizep = size; } } - if (disk_sizep) { + if (oi->disk_sizep) { struct revindex_entry *revidx = find_pack_revindex(p, obj_offset); - *disk_sizep = revidx[1].offset - obj_offset; + *oi->disk_sizep = revidx[1].offset - obj_offset; } - if (typep) { - *typep = packed_to_object_type(p, obj_offset, type, &w_curs, curpos); - if (*typep < 0) { + if (oi->typep) { + *oi->typep = packed_to_object_type(p, obj_offset, type, &w_curs, curpos); + if (*oi->typep < 0) { type = OBJ_BAD; goto out; } @@ -2385,9 +2384,7 @@ struct packed_git *find_sha1_pack(const unsigned char *sha1, } static int sha1_loose_object_info(const unsigned char *sha1, - enum object_type *typep, - unsigned long *sizep, - unsigned long *disk_sizep) + struct object_info *oi) { int status; unsigned long mapsize, size; @@ -2399,12 +2396,12 @@ static int sha1_loose_object_info(const unsigned char *sha1, * If we don't care about type or size, then we don't * need to look inside the object at all. */ - if (!typep && !sizep) { - if (disk_sizep) { + if (!oi->typep && !oi->sizep) { + if (oi->disk_sizep) { struct stat st; if (stat_sha1_file(sha1, &st) < 0) return -1; - *disk_sizep = st.st_size; + *oi->disk_sizep = st.st_size; } return 0; } @@ -2412,19 +2409,19 @@ static int sha1_loose_object_info(const unsigned char *sha1, map = map_sha1_file(sha1, &mapsize); if (!map) return error("unable to find %s", sha1_to_hex(sha1)); - if (disk_sizep) - *disk_sizep = mapsize; + if (oi->disk_sizep) + *oi->disk_sizep = mapsize; if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0) status = error("unable to unpack %s header", sha1_to_hex(sha1)); else if ((status = parse_sha1_header(hdr, &size)) < 0) status = error("unable to parse %s header", sha1_to_hex(sha1)); - else if (sizep) - *sizep = size; + else if (oi->sizep) + *oi->sizep = size; git_inflate_end(&stream); munmap(map, mapsize); - if (typep) - *typep = status; + if (oi->typep) + *oi->typep = status; return 0; } @@ -2449,8 +2446,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) if (!find_pack_entry(sha1, &e)) { /* Most likely it's a loose object. */ - if (!sha1_loose_object_info(sha1, oi->typep, - oi->sizep, oi->disk_sizep)) { + if (!sha1_loose_object_info(sha1, oi)) { oi->whence = OI_LOOSE; return 0; } @@ -2461,8 +2457,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) return -1; } - rtype = packed_object_info(e.p, e.offset, oi->typep, oi->sizep, - oi->disk_sizep); + rtype = packed_object_info(e.p, e.offset, oi); if (rtype < 0) { mark_bad_packed_object(e.p, sha1); return sha1_object_info_extended(sha1, oi); From d099b7173dabdeeb1f339151ac2169b3a91bf631 Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Thu, 18 Jul 2013 21:25:50 +0100 Subject: [PATCH 8/8] Fix some sparse warnings Sparse issues some "Using plain integer as NULL pointer" warnings. Each warning relates to the use of an '{0}' initialiser expression in the declaration of an 'struct object_info'. The first field of this structure has pointer type. Thus, in order to suppress these warnings, we replace the initialiser expression with '{NULL}'. Signed-off-by: Ramsay Jones Acked-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_file.c | 2 +- streaming.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 2500b94478..452e4647b3 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2477,7 +2477,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) { enum object_type type; - struct object_info oi = {0}; + struct object_info oi = {NULL}; oi.typep = &type; oi.sizep = sizep; diff --git a/streaming.c b/streaming.c index 870657ab56..acc07a6ff1 100644 --- a/streaming.c +++ b/streaming.c @@ -135,7 +135,7 @@ struct git_istream *open_istream(const unsigned char *sha1, struct stream_filter *filter) { struct git_istream *st; - struct object_info oi = {0}; + struct object_info oi = {NULL}; const unsigned char *real = lookup_replace_object(sha1); enum input_source src = istream_source(real, type, &oi);