Re: [PATCH] use persistent memory for rejected paths

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 15, 2010, 18:31 UTC
Message-ID: <7vvd3yl9q2.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20101114130205.GA27560@localhost>

Clemens Buchacher <drizzd@aon.at> writes:

Show 6 quoted lines

> An aborted merge prints the list of rejected paths as part of the
> error message. Some of those paths do not have static buffers, so
> we have to keep a copy. Use string_list's to accomplish this.
>
> Previous to this fix, the error message would print whatever was
> stored in the stack at that point.

Hmmm, all calls to add_rejected_path() seems to be with ce->name as the path parameter, and I do not think we ever free cache entries (either taken from the index or synthesized during the merge), so I am a bit surprised that this is necessary (namely, if some ce->name points into the stack, wouldn't that be a more serious bug than misreporting???).

> With this change, the path list is printed in the order of
> processing.  Previously, the order was reversed.

That is true but I wonder if the order should be "whatever the processing order happens to be" in the first place, as this is a report to the end user, no? Perhaps "collect in strlist, sort at the end before showing" is a more desirable thing to do?

Still, I am more disturbed by the "some do not have static"...