Re: [PATCH v20 43/48] refs.c: move the check for valid refname to lock_ref_sha1_basic

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

 



On Tue, Jul 15, 2014 at 11:04 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Michael Haggerty wrote:
>
>> So...I like the idea of enforcing refname checks at the lowest level
>> possible, but I think that the change you propose is too abrupt.  I
>> think it needs either more careful analysis showing that it won't hurt
>> anybody, or some kind of tooling or non-strict mode that people can use
>> to fix their repositories.
>
> The recovery code has been broken for a while, so I don't see harm
> in this change.
>
> How to take care of the recovery use case is another question.  FWIW I
> also would prefer if "git update-ref -d" or "git branch -D" could be
> used to delete corrupt refs instead of having to use fsck (since a
> fsck run can take a while), but that's a question for a later series.
>
> In an ideal world, the low-level functions would allow *reading* and
> *deleting* poorly named refs (even without any special flag) but not
> creating them.  Is that doable?

That should be doable. Ill add these repairs as 3-4 new patches at the
end of the current patch series.

> The main complication I can see is
> iteration: would iteration skip poorly named refs and warn, or would
> something more complicated be needed?

Right now,  "git branch"  will error and abort right away when it
finds the first bad ref. I haven't checked the exact spot it does this
yet but I suspect it is when building the ref-cache.
I will solve the cases for create and delete for now.


What/how to handle iterables will require more thought.
Right now, since these refs will be filtered out and never end up in
ref-cache, either from loose refs or from packed refs
it does mean that anyone that uses an iterator is guaranteed to only
get refs with valid names passed back to them.
We would need to audit all code that uses iterators and make sure it
can handle the case where the callback is suddenly
invoked with a bad refname.

>
> Thanks,
> Jonathan
--
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]