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:
>
> #define READ_SHA1_FILE_REPLACE 1
> 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,
> READ_SHA1_FILE_REPLACE);
> }
>
> 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.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html