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

[Michael Haggerty <mhagger@alum.mit.edu>]

On 05/07/2013 04:39 AM, Jeff King wrote:
> 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

> 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/