git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] find_pack_entry(): do not keep packed_git pointer locally

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 30, 2012, 23:26 UTC
Message-ID
<7v62fsai1i.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1327922750-12106-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 12 quoted lines
> 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@gmail.com>
> ---
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?
Show 58 quoted lines
> 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;
Previous: Nguyễn Thái Ngọc DuyNext: Nguyen Thai Ngoc Duy
Message 2 of 14 in “find_pack_entry(): do not keep packed_git pointer locally”
  1. find_pack_entry(): do not keep packed_git pointer locallyNguyễn Thái Ngọc Duy, Jan 30, 2012
  2. Junio C HamanoJan 30, 2012
  3. Nguyen Thai Ngoc DuyJan 31, 2012
  4. Nicolas PitreJan 31, 2012
  5. Junio C HamanoJan 31, 2012
  6. Nicolas PitreJan 31, 2012
  7. 1/2 Factor find_pack_entry()'s core outNguyễn Thái Ngọc Duy, Feb 1, 2012
  8. 2/2 find_pack_entry(): do not keep packed_git pointer locallyNguyễn Thái Ngọc Duy, Feb 1, 2012
  9. Nicolas PitreFeb 1, 2012
  10. 2/2 find_pack_entry(): do not keep packed_git pointer locallyNguyễn Thái Ngọc Duy, Feb 2, 2012
  11. Nicolas PitreFeb 1, 2012
  12. Junio C HamanoFeb 1, 2012
  13. Junio C HamanoFeb 1, 2012
  14. Nicolas PitreFeb 1, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.