Re: [PATCH v4 5/8] sha1_file: refactor read_object
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 21, 2017, 17:58 UTC
- Message-ID
- <xmqqefudql37.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <73d085b6fad9316c5a500341db2dd6451700656e.1497920092.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 23 quoted lines
> @@ -2914,6 +2912,7 @@ static int sha1_loose_object_info(const unsigned char *sha1,
> git_zstream stream;
> char hdr[32];
> struct strbuf hdrbuf = STRBUF_INIT;
> + unsigned long size_scratch;
>
> if (oi->delta_base_sha1)
> hashclr(oi->delta_base_sha1);
> @@ -2939,6 +2938,10 @@ static int sha1_loose_object_info(const unsigned char *sha1,
> map = map_sha1_file(sha1, &mapsize);
> if (!map)
> return -1;
> +
> + if (!oi->sizep)
> + oi->sizep = &size_scratch;
> +
> if (oi->disk_sizep)
> *oi->disk_sizep = mapsize;
> if ((flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE)) {
> if (status && oi->typep)
> *oi->typep = status;
> + if (oi->sizep == &size_scratch)
> + oi->sizep = NULL;This looked somewhat unusual but nevertheless is correct. Because of the way parse_sha1_header_extended() interacts with its callers, the usual fn(oi->sizep ? oi->sizep : &dummy) pattern does not apply to this codepath.
Show 35 quoted lines
> @@ -3077,28 +3090,15 @@ int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
> static void *read_object(const unsigned char *sha1, enum object_type *type,
> unsigned long *size)
> {
> - unsigned long mapsize;
> - void *map, *buf;
> - struct cached_object *co;
> -
> - co = find_cached_object(sha1);
> - if (co) {
> - *type = co->type;
> - *size = co->size;
> - return xmemdupz(co->buf, co->size);
> - }
> + struct object_info oi = OBJECT_INFO_INIT;
> + void *content;
> + oi.typep = type;
> + oi.sizep = size;
> + oi.contentp = &content;
>
> - buf = read_packed_sha1(sha1, type, size);
> - if (buf)
> - return buf;
> - map = map_sha1_file(sha1, &mapsize);
> - if (map) {
> - buf = unpack_sha1_file(map, mapsize, type, size, sha1);
> - munmap(map, mapsize);
> - return buf;
> - }
> - reprepare_packed_git();
> - return read_packed_sha1(sha1, type, size);
> + if (sha1_object_info_extended(sha1, &oi, 0))
> + return NULL;
> + return content;
> }Nice code reduction; it is somewhat funny to think that a function meant to gather 'object info' does so much, but we can always say the contents is part of the information about the object ;-).
Same comment as the other one applies here; the definition of how an error is reported by sha1_object_info_extended() should be kept consistent with existing callers.
Thanks.