On Tue, Feb 14, 2017 at 8:55 PM, Jeff King <peff@xxxxxxxx> wrote: > On Tue, Feb 14, 2017 at 11:35:48AM -0800, Junio C Hamano wrote: > >> Jeff King <peff@xxxxxxxx> writes: >> >> > This fixes the problem, but I think we can simplify it quite a bit by >> > using resolve_refdup(). Here's the patch series I ended up with: >> > >> > [1/3]: show-branch: drop head_len variable >> > [2/3]: show-branch: store resolved head in heap buffer >> > [3/3]: show-branch: use skip_prefix to drop magic numbers Yeah, I noticed there were a number of things that could be improved in the area, but I didn't want to spend too much time on this, so thanks for this series. >> Yes, the whole thing is my fault ;-) and I agree with what these >> patches do. >> >> The second one lacks free(head) but I think that is OK; it is >> something we allocate in cmd_*() and use pretty much thruout the >> rest of the program. > > Yes, I actually tested the whole thing under ASAN (which was necessary > to notice the problem), which complained about the leak. I don't mind > adding a free(head), but there are a bunch of similar "leaks" in that > function, so I didn't bother. Yeah, I didn't bother either. > I notice Christian's patch added a few tests. I don't know if we'd want > to squash them in (I didn't mean to override his patch at all; I was > about to send mine out when I noticed his, and I wondered if we wanted > to combine the two efforts). I think it would be nice to have at least one test. Feel free to squash mine if you want.