Junio C Hamano <gitster@xxxxxxxxx> wrote: > Eric Wong <e@xxxxxxxxx> writes: > > > We need to check delta_base_cache anyways to fill in the > > `whence' field in `struct object_info'. Inlining (and getting > > rid of) cache_or_unpack_entry() makes it easier to only do the > > hashmap lookup once and avoid a redundant lookup later on. > > > > This code reorganization will also make an optimization to > > use the cache entry directly easier to implement in the next > > commit. > > "cache entry" -> "cached entry"; we tend to use "cache entry" > exclusively to mean an entry in the in-core index structure, > and not the cached objects held in the object layer. OK, will do for v3 (apologies for the delay, Real Life is awful :<). > > + type = ent->type; > > + if (oi->sizep) > > + *oi->sizep = ent->size; > > + if (oi->contentp) { > > + if (!oi->content_limit || > > + ent->size <= oi->content_limit) > > + *oi->contentp = xmemdupz(ent->data, ent->size); > > + else > > + *oi->contentp = NULL; /* caller must stream */ > > This assignment of NULL is more explicit than the original; is it > because the original assumed that *(oi->contentp) is initialized to > NULL if oi->contentp asks us to give the contents? The original code unconditionally assigned cache_or_unpack_entry() result to *oi->contentp, and there's a bunch of non-cat-file callers which pass a non-NULL *oi->contentp and expect packed_object_info() to NULL it.