On Fri, Nov 22, 2013 at 12:04:01PM +0100, Christian Couder wrote:

> In "sha1_file.c", there is:
> void *read_sha1_file_extended(const unsigned char *sha1,
>                               enum object_type *type,
>                               unsigned long *size,
>                               unsigned flag)
> {
>         void *data;
>         char *path;
>         const struct packed_git *p;
>         const unsigned char *repl = (flag & READ_SHA1_FILE_REPLACE)
>                 ? lookup_replace_object(sha1) : sha1;
>         errno = 0;
>         data = read_object(repl, type, size);
> ...
> And in cache.h, there is:
> static inline void *read_sha1_file(const unsigned char *sha1, enum
> object_type *type, unsigned long *size)
> {
>         return read_sha1_file_extended(sha1, type, size,
> }
> So the READ_SHA1_FILE_REPLACE is a way to disable replacement at compile time.

Is it? I did not have the impression anyone would ever redefine
READ_SHA1_FILE_REPLACE at compile time, but that it was a flag that
individual callsites would pass to read_sha1_file_extended to tell them
whether they were interested in replacements. And the obvious reasons to
not be are:

  1. You are doing some operation which needs real objects, like fsck or
     generating a packfile.

  2. You have already resolved any replacements, and want to make sure
     you are getting the same object used elsewhere (e.g., because you
     already printed its size :) ).

The only site which calls read_sha1_file_extended directly and does not
pass the REPLACE flag is in streaming.c. And that looks to be a case of
(2), since we resolve the replacement at the start in open_istream().

I am kind of surprised we do not need to do so for (1) in places like
pack-objects.c. Most of that code does not use read_sha1_file,
preferring instead to find the individual pack entries (for reuse). But
there are some calls to read_sha1_file, and I wonder if there is a bug
lurking there.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to