Re: [PATCH 28/31] dir: make untracked cache extension hash size independent

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 12, 2019, 11:08 UTC
Message-ID: <87y36ljmcu.fsf@evledraar.gmail.com>
In-Reply-To: <20190212012256.1005924-29-sandals@crustytoothpaste.net>

On Tue, Feb 12 2019, brian m. carlson wrote:

Show 11 quoted lines

> Instead of using a struct with a flex array member to read and write the
> untracked cache extension, use a shorter, fixed-length struct and add
> the name and hash data explicitly.
> [...]
>  	struct stat_data info_exclude_stat;
>  	struct stat_data excludes_file_stat;
>  	uint32_t dir_flags;
> -	unsigned char info_exclude_sha1[20];
> -	unsigned char excludes_file_sha1[20];
> -	char exclude_per_dir[FLEX_ARRAY];
>  };

Both this & the follow-up 29/31 look scary since this is an on-disk structure and this patch & the next one rather than implementing some transition, just changes the structs & code we use to read & write to use the current hash size.

So what are we going to do when the "current" size is SHA-256 and our on-disk cache is SHA-1? It seems like with this we'd at best (I haven't tested) throw away the SHA-1 UC data, and at worst introduce some silent persistent read failure.

In any case that seems like something we should have tests for with an on-disk format, i.e. write in one hash, see what happens when we read in another, and perhaps instead of not understanding SHA-1 hashes in SHA-256 mode we'd read the SHA-1 and consult the SHA-1<->SHA-256 lookup table we're going to be eventually maintaining on the side?