Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> Commit f7c22cc (always start looking up objects in the last used pack
> first - 2007-05-30) introduces a static packed_git* pointer as an
> optimization. The kept pointer however may become invalid if
> free_pack_by_name() happens to free that particular pack.
>
> Current code base does not access packs after calling
> free_pack_by_name() so it should not be a problem. Anyway, move the
> pointer out so that free_pack_by_name() can reset it to avoid running
> into troubles in future.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
Thanks. Two curiosities:
- Why is there no hunk to actually clear the pointer in
free_pack_by_name() in this patch?
- Could we make the magic (void *)1 value a #define'd constant? Perhaps
we could even use NULL for that purpose?
> diff --git a/sha1_file.c b/sha1_file.c
> index 88f2151..4ecc953 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -54,6 +54,8 @@ static struct cached_object empty_tree = {
> 0
> };
>
> +static struct packed_git *find_pack_entry_last_found = (void *)1;
> +
> static struct cached_object *find_cached_object(const unsigned char *sha1)
> {
> int i;
> @@ -720,6 +722,8 @@ void free_pack_by_name(const char *pack_name)
> close_pack_index(p);
> free(p->bad_object_sha1);
> *pp = p->next;
> + if (find_pack_entry_last_found == p)
> + find_pack_entry_last_found = (void*)1;
> free(p);
> return;
> }
> @@ -2012,14 +2016,13 @@ int is_pack_valid(struct packed_git *p)
>
> static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
> {
> - static struct packed_git *last_found = (void *)1;
> struct packed_git *p;
> off_t offset;
>
> prepare_packed_git();
> if (!packed_git)
> return 0;
> - p = (last_found == (void *)1) ? packed_git : last_found;
> + p = (find_pack_entry_last_found == (void *)1) ? packed_git : find_pack_entry_last_found;
>
> do {
> if (p->num_bad_objects) {
> @@ -2046,16 +2049,16 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
> e->offset = offset;
> e->p = p;
> hashcpy(e->sha1, sha1);
> - last_found = p;
> + find_pack_entry_last_found = p;
> return 1;
> }
>
> next:
> - if (p == last_found)
> + if (p == find_pack_entry_last_found)
> p = packed_git;
> else
> p = p->next;
> - if (p == last_found)
> + if (p == find_pack_entry_last_found)
> p = p->next;
> } while (p);
> return 0;
--
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