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

Re: [PATCH 2/4] add a stat_validity struct

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
May 13, 2013, 02:29 UTC
Message-ID
<5190500E.8060907@alum.mit.edu>
In-Reply-To
<20130507023942.GB22940@sigill.intra.peff.net>
On 05/07/2013 04:39 AM, Jeff King wrote:
Show 12 quoted lines
> It can sometimes be useful to know whether a path in the
> filesystem has been updated without going to the work of
> opening and re-reading its content. We trust the stat()
> information on disk already to handle index updates, and we
> can use the same trick here.
> 
> This patch introduces a "stat_validity" struct which
> encapsulates the concept of checking the stat-freshness of a
> file. It is implemented on top of "struct cache_entry" to
> reuse the logic about which stat entries to trust for a
> particular platform, but hides the complexity behind two
> simple functions: check and update.

Given that "struct cache_entry" holds a lot of information that is irrelevant to stat-freshness and that ce_match_stat_basic() has a lot of logic that is geared towards cache_entries, it seems like there should be some kind of "struct stat_data" that holds the portion of the stat information that we use for checking whether a file might have been changed between two accesses. cache_entry would contain a stat_data as a member, and the functionality for checking that part of a cache_entry validity would be moved to a separate function.

Then your "struct validity_info" could hold a pointer to a stat_data rather than to a cache_entry, and it wouldn't have to contain the unrelated cache_entry stuff.

I'll send patches shortly to show what I mean.
Michael
Show 89 quoted lines
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This one is prep for the next patch. I'm not super-happy with the way it
> builds around cache_entry, just because cache entries may end up
> following different rules in the long run. But I at least tried to
> encapsulate the grossness, so if it turns out to be a problem, we can
> factor out the relevant bits from ce_match_stat_basic into a shared
> function.
> 
>  cache.h      | 27 +++++++++++++++++++++++++++
>  read-cache.c | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/cache.h b/cache.h
> index 94ca1ac..adf2874 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1326,4 +1326,31 @@ int sane_execvp(const char *file, char *const argv[]);
>  
>  int sane_execvp(const char *file, char *const argv[]);
>  
> +/*
> + * A struct to encapsulate the concept of whether a file has changed
> + * since we last checked it. This is a simplified version of the up-to-date
> + * checks we use for the index. The implementation is built on an index entry,
> + * but we shield the callers from that ugliness with our struct.
> + */
> +struct stat_validity {
> +	struct cache_entry *ce;
> +};
> +
> +void stat_validity_clear(struct stat_validity *sv);
> +
> +/*
> + * Returns 1 if the path matches the saved stat_validity, 0 otherwise.
> + * A missing or inaccessible file is considered a match if the struct was just
> + * initialized, or if the previous update found an inaccessible file.
> + */
> +int stat_validity_check(struct stat_validity *sv, const char *path);
> +
> +/*
> + * Update the stat_validity from a file opened at descriptor fd (if the file
> + * is missing or inaccessible, the validity will reflect that, and future
> + * calls to stat_validity_check will match only if it continues to be
> + * inaccessible).
> + */
> +void stat_validity_update(struct stat_validity *sv, int fd);
> +
>  #endif /* CACHE_H */
> diff --git a/read-cache.c b/read-cache.c
> index 04ed561..a0bd06c 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1933,3 +1933,34 @@ void *read_blob_data_from_index(struct index_state *istate, const char *path, un
>  		*size = sz;
>  	return data;
>  }
> +
> +void stat_validity_clear(struct stat_validity *sv)
> +{
> +	free(sv->ce);
> +	sv->ce = NULL;
> +}
> +
> +int stat_validity_check(struct stat_validity *sv, const char *path)
> +{
> +	struct stat st;
> +
> +	if (stat(path, &st) < 0)
> +		return sv->ce == NULL;
> +	if (!sv->ce)
> +		return 0;
> +	return !ce_match_stat_basic(sv->ce, &st);
> +}
> +
> +void stat_validity_update(struct stat_validity *sv, int fd)
> +{
> +	struct stat st;
> +
> +	if (fstat(fd, &st) < 0)
> +		stat_validity_clear(sv);
> +	else {
> +		if (!sv->ce)
> +			sv->ce = xcalloc(1, cache_entry_size(0));
> +		fill_stat_cache_info(sv->ce, &st);
> +		sv->ce->ce_mode = create_ce_mode(st.st_mode);
> +	}
> +}
> 
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Previous: Jeff KingNext: Michael Haggerty
Message 31 of 45 in “another packed-refs race”
  1. Jeff KingMay 3, 2013
  2. Johan HerlandMay 3, 2013
  3. Jeff KingMay 3, 2013
  4. Jeff KingMay 3, 2013
  5. Johan HerlandMay 3, 2013
  6. Michael HaggertyMay 6, 2013
  7. Jeff KingMay 6, 2013
  8. Jeff KingMay 3, 2013
  9. Michael HaggertyMay 6, 2013
  10. Jeff KingMay 6, 2013
  11. Jeff KingMay 6, 2013
  12. Michael HaggertyMay 7, 2013
  13. Jeff KingMay 7, 2013
  14. Michael HaggertyMay 7, 2013
  15. 0/4 fix packed-refs racesJeff King, May 7, 2013
  16. 1/4 resolve_ref: close race condition for packed refsJeff King, May 7, 2013
  17. Michael HaggertyMay 12, 2013
  18. Jeff KingMay 16, 2013
  19. Michael HaggertyMay 16, 2013
  20. Michael HaggertyMay 12, 2013
  21. 0/4 Fix a race condition when reading loose refsMichael Haggerty, Jun 11, 2013
  22. 1/4 resolve_ref_unsafe(): extract function handle_missing_loose_ref()Michael Haggerty, Jun 11, 2013
  23. 2/4 resolve_ref_unsafe(): handle the case of an SHA-1 within loopMichael Haggerty, Jun 11, 2013
  24. 3/4 resolve_ref_unsafe(): nest reference-reading code in an infinite loopMichael Haggerty, Jun 11, 2013
  25. 4/4 resolve_ref_unsafe(): close race condition reading loose refsMichael Haggerty, Jun 11, 2013
  26. Jeff KingJun 12, 2013
  27. Thomas RastJun 13, 2013
  28. Michael HaggertyJun 14, 2013
  29. Junio C HamanoJun 11, 2013
  30. 2/4 add a stat_validity structJeff King, May 7, 2013
  31. Michael HaggertyMay 13, 2013
  32. 0/2 Separate stat_data from cache_entryMichael Haggerty, May 13, 2013
  33. 1/2 Extract a struct stat_data from cache_entryMichael Haggerty, May 13, 2013
  34. 2/2 add a stat_validity structMichael Haggerty, May 13, 2013
  35. Junio C HamanoMay 13, 2013
  36. Jeff KingMay 16, 2013
  37. 3/4 get_packed_refs: reload packed-refs file when it changesJeff King, May 7, 2013
  38. 0/2 peel_ref cleanups changesJeff King, May 7, 2013
  39. 1/2 peel_ref: rename "sha1" argument to "peeled"Jeff King, May 7, 2013
  40. 2/2 peel_ref: refactor for safety with simultaneous updateJeff King, May 7, 2013
  41. Eric SunshineMay 9, 2013
  42. Michael HaggertyMay 13, 2013
  43. 4/4 for_each_ref: load all loose refs before packed refsJeff King, May 7, 2013
  44. Junio C HamanoMay 7, 2013
  45. Jeff KingMay 7, 2013

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.