I hadn't thought of exploiting the fact that the SHA1 of an empty file
is fixed. Nice! I believe I can prove there are no races now.
Incidentally, this is how I first found the bug: I was trying to prove
what git did worked.
I still prefer a per-entry flag solution (I suspect it's faster, and
the proof is easier), but that's too much work.
-Ben
On Tue, Jun 10, 2008 at 10:44 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>
> On Tue, 10 Jun 2008, Ben Lynn wrote:
>>
>> Unfortunately, the solution isn't perfect. Try this:
>
> Heh.
>
> That's just because our "smudge_racily_clean_entry()" uses 0 as the magic
> smudging size.
>
> You can fix this multiple ways. One would be to pick another size that is
> simply less likely (eg ~0 instead), which leaves the theoretical race, and
> just makes it practically impossible to hit (not that I think it's very
> practical to hit already).
>
> The other approach is to know that an empty blob always has a very
> specific SHA1. Here's an trial patch.
>
> Linus
>
> ---
> read-cache.c | 16 ++++++++++++++++
> 1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 8e5fbb6..f83de8c 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -138,6 +138,16 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
> return 0;
> }
>
> +static int is_empty_blob_sha1(const unsigned char *sha1)
> +{
> + static const unsigned char empty_blob_sha1[20] = {
> + 0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
> + 0x29,0xae,0x77,0x5a,0xd8,0xc2,0xe4,0x8c,0x53,0x91
> + };
> +
> + return !hashcmp(sha1, empty_blob_sha1);
> +}
> +
> static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
> {
> unsigned int changed = 0;
> @@ -193,6 +203,12 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
> if (ce->ce_size != (unsigned int) st->st_size)
> changed |= DATA_CHANGED;
>
> + /* Racily smudged entry? */
> + if (!ce->ce_size) {
> + if (!is_empty_blob_sha1(ce->sha1))
> + changed |= DATA_CHANGED;
> + }
> +
> return changed;
> }
>
>
--
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
