On Tue, 17 Apr 2007, Junio C Hamano wrote:
I wonder why the loss of "we are replacing the same one" case in the original add_ref() was not compensated for in the new sort_ref_list().
That would be because it didn't bite me during testing, and I forgot to double check the code I was removing after I was happy that the sort was producing sane output (it was getting rather late).
Thing is, I'm not sure what to do about it anyway. You could drop the duplicate in the sort, but since add_ref returns a pointer to the added entry (and used to simply return a pointer to the one already in the list) it's possible that you would be dropping something that someone had a pointer to.
I think we would not call add_ref() to the same list with duplicate names, unless (1) filesystem is grossly corrupt, (2) somebody added a new ref while we are walking (how does readdir() behave in such a case???), or (3) packed-refs file is corrupt.
This combined with the fact that the old code didn't check that the sha1 was the same suggests to me that this behaviour may actually have been a subtle bug? Perhaps the best thing to do is die if we find two entries with the same name when sorting?
-- Julian --- What passes for optimism is most often the effect of an intellectual error. -- Raymond Aron, "The Opium of the Intellectuals" - 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