git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Julian Phillips <julian@quantumfyre.co.uk>
To: Junio C Hamano <junkio@cox.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>, git@vger.kernel.org
Subject: Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add
Date: Tue, 17 Apr 2007 23:43:30 +0100 (BST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0704172304380.16435@beast.quantumfyre.co.uk> (raw)
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.

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

  reply	other threads:[~2007-04-17 22:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-17  1:42 [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add Julian Phillips
2007-04-17 16:03 ` Linus Torvalds
2007-04-17 20:17   ` Julian Phillips
2007-04-17 20:51     ` Linus Torvalds
2007-04-17 22:03       ` Julian Phillips
2007-04-17 22:00   ` Junio C Hamano
2007-04-17 22:43     ` Julian Phillips [this message]
2007-04-18 19:36       ` Junio C Hamano
2007-04-18 21:27         ` [PATCH] refs.c: drop duplicate entries in sort_ref_list Julian Phillips
2007-04-19  8:19           ` Johannes Sixt
2007-04-19  8:49             ` Julian Phillips
2007-04-17 21:01 ` [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add Julian Phillips

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0704172304380.16435@beast.quantumfyre.co.uk \
    --to=julian@quantumfyre.co.uk \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).