On Tue, 10 Feb 2009, Shawn O. Pearce wrote:
> Daniel Barkalow <barkalow@xxxxxxxxxxxx> wrote:
> > On Tue, 10 Feb 2009, Shawn O. Pearce wrote:
> > >
> > > We should dump the cached_objects table in sha1_file.c during
> > > a checkpoint in fast-import.
> >
> > No, that one's keyed by sha1, and doesn't get collisions; it's the
> > delta_base_cache that's the issue; it's keyed by struct packed_git * and
> > offset.
>
> Uh, yea, I realize that after I sent the message. Does this patch
> fix it for you?
Aside from the trivial typo, yes. (Although I can't be 100% sure it didn't
just happen to change things leading to needing a different test case; I
can say for sure that it got past the previous code's MTBF, which is a
good sign.)
> --8<--
> Clear the delta base cache during fast-import checkpoint
>
> Otherwise we may reuse the same memory address for a totally
> different "struct packed_git", and a previously cached object from
> the prior occupant might be returned when trying to unpack an object
> from the new pack.
>
> Found-by: Daniel Barkalow <barkalow@xxxxxxxxxxxx>
> Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>
> ---
> cache.h | 1 +
> fast-import.c | 1 +
> sha1_file.c | 7 +++++++
> 3 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 8dcc53c..7f1a6e8 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -830,6 +830,7 @@ extern unsigned char* use_pack(struct packed_git *, struct pack_window **, off_t
> extern void close_pack_windows(struct packed_git *);
> extern void unuse_pack(struct pack_window **);
> extern void free_pack_by_name(const char *);
> +extern void clear_delta_base_cache(void);
> extern struct packed_git *add_packed_git(const char *, int, int);
> extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t);
> extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);
> diff --git a/fast-import.c b/fast-import.c
> index 1935206..03b13e0 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -945,6 +945,7 @@ static void end_packfile(void)
> {
> struct packed_git *old_p = pack_data, *new_p;
>
> + clear_delta_base_cache();
> if (object_count) {
> char *idx_name;
> int i;
> diff --git a/sha1_file.c b/sha1_file.c
> index 8868b80..d2dbc96 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1663,6 +1663,13 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)
> }
> }
>
> +void clear_delta_base_cache(void)
> +{
> + unsigned long p;
> + for (p = 0; p < MAX_DELTA_CACHE; p++)
> + release_delta_base_cache(&delta_base_cache[p])
> +}
> +
> static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
> void *base, unsigned long base_size, enum object_type type)
> {
> --
> 1.6.2.rc0.186.g417c
>
> --
> 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
>
--
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
