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

[Jeff King <peff@peff.net>]

On Mon, May 13, 2013 at 04:29:34AM +0200, Michael Haggerty wrote:

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

Yes, I had the exact same thought, and came up with the exact same
solution. I was just hesitant to do it because it meant touching all of
the cache_entry users to adjust their struct accesses.

But from your patches, it doesn't look too bad (and we can be sure we
updated every spot, because it's easy for the compiler to enforce).

I'm sorry to be so slow lately; I'm on vacation (and will be for a bit
yet). I'm happy to rebuild my series on top of yours, and it looks like
it is still only in pu (I haven't yet read "What's Cooking" to see the
plans for it). I don't think there's a rush, as it is post-1.8.3 anyway
(I _do_ think it's a serious bug, in that it can cause data loss, but in
practice I doubt most people would hit it).

-Peff