On Wed, Jun 03, 2015 at 03:51:59PM +0200, Michael Haggerty wrote:
> NULL_SHA1 is used to indicate of "invalid SHA-1" throughout our code
s/of/an/ ?
> (and the code of other git implementations), so it is vastly more
> likely that a reference was set to this value due to a software bug
> than that NULL_SHA1 is the legitimate SHA-1 of an actual object.
> Therefore, if a loose reference has the value NULL_SHA1, consider it
> to be broken.
>
> Amusingly, each of the other 2^160-1 possible SHA-1 values is exactly
> as unlikely as NULL_SHA1 to be the SHA-1 of an actual object. The
> difference is that most of those other values are also very unlikely
> to be written to a loose reference file by accident, whereas
> accidentally writing NULL_SHA1 to a loose reference file would be an
> easy mistake to make.
FWIW, I think this justification (and the comment below) reads better
than what you had before.
> diff --git a/refs.c b/refs.c
> index 6736424..83af13d 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1324,6 +1324,16 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
> if (!read_ok) {
> hashclr(sha1);
> flag |= REF_ISBROKEN;
> + } else if (is_null_sha1(sha1)) {
> + /*
> + * It is so astronomically unlikely
> + * that NULL_SHA1 is the SHA-1 of an
> + * actual object that we consider its
> + * appearance in a loose reference
> + * file to be repo corruption
> + * (probably due to a software bug).
> + */
> + flag |= REF_ISBROKEN;
Nice. After reading the other thread, I did not think we needed to
bother with more refactoring here, but I agree this end result flows
nicely.
-Peff
--
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