Re: [PATCH 0/2] Fix a refname trimming problem in `log --bisect`

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

 



Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:

> The code for `git log --bisect` was calling
> `for_each_ref_in_submodule()` with prefix set to "refs/bisect/bad",
> which is the actual name of the reference that it wants. This resulted
> in the refname being trimmed completely away and the empty string
> being passed to the callback. That became impermissible after
>
>     b9c8e7f2fb prefix_ref_iterator: don't trim too much, 2017-05-22
>
> , so the command was failing.
>
> Fix the problem in two orthogonal ways:
>
> 1. Add a new function, `for_each_fullref_in_submodule()`, that doesn't
>    trim the refnames that it passes to callbacks, and us that instead.
>    I *think* that this is a strict improvement, though I don't know
>    the `git log` code well enough to be sure that it won't have bad
>    side-effects.
>
> 2. Relax the "trimming too many characters" check to allow the full
>    length of the refname to be trimmed away (though not more than
>    that).
>
> In an ideal world the second patch shouldn't be necessary, because
> this calling pattern is questionable and it might be better that we
> learn about any other offenders. But if we'd rather be conservative
> and not break any other code that might rely on the old behavior,
> patch 2 is my suggestion for how to do it.

Thanks for a nice summary.  

I agree that 2. is a nice safety to have, especially if the code
before b9c8e7f2 ("prefix_ref_iterator: don't trim too much",
2017-05-22) has been seeing the completely trimmed result (i.e. an
empty string) in the callback function.

And I agree that 1. is also a good interface to have.



[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]