Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add

From: Julian Phillips <julian@quantumfyre.co.uk>
Date: Apr 17, 2007, 22:43 UTC
Message-ID: <Pine.LNX.4.64.0704172304380.16435@beast.quantumfyre.co.uk>
In-Reply-To: <7vslaymzk3.fsf@assigned-by-dhcp.cox.net>

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.

Show 5 quoted lines

> 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"