Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> At any rate, I think the changes should be all or nothing. If the
> warning goes away, fine. But if the warning stays, and dwim_ref is going
> to have special rules for looking in the top-level $GIT_DIR, then things
> like shorten_unambiguous_ref need to respect those rules, or we've just
> created a new bug.

Whether we remove the warning or not, I think it would be an improvement
not to look at random files directly underneath $GIT_DIR/. I am not sure
how we can be confident that we caught everything, though.

In other words, is shorten-unambiguous-refs the last one that needs
fixing? How would we know for certain?

Also I tend to think Michael's "only warn in refs/" is probably not the
right solution. When a caller asks to resolve_ref(MERGE_HEAD), one of
these things can be true:

 - A file $GIT_DIR/MERGE_HEAD does not exist; this is not inherently an
   error unless we were supposed to be in the middle of a conflicted
   merge.

 - A file $GIT_DIR/MERGE_HEAD exists, and records a correct 40-hexadecimal
   get_sha1_hex() can grok. This is perfectly normal.

 - A file $GIT_DIR/MERGE_HEAD exists, but get_sha1_hex() does not grok
   it. Michael warns against this twice, and I think it is a wrong thing
   to pass this unnoticed.

Once we tighten all the "too loose accesses to $GIT_DIR/$random_filename",
we might even want to have an option to cause the caller to die() in the
error case, and the logic is the same for refs under $GIT_DIR/refs/, not
just the ref-like-things directly under $GIT_DIR.

A regular ref, can also appear in $GIT_DIR/packed-refs, but a corruption
of an entry in the file will be caught when the file is read and outside
the scope of this discussion, I think.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]