From: Duy Nguyen Date: Wed, 27 Mar 2013 01:40:16 GMT Subject: Re: [PATCH 1/4] attr.c::path_matches(): the basename is part of the pathname Message-ID: In-Reply-To: <20130326184921.GA26462@sigill.intra.peff.net> On Wed, Mar 27, 2013 at 1:49 AM, Jeff King wrote: > On Tue, Mar 26, 2013 at 11:39:28AM -0700, Junio C Hamano wrote: > >> The function takes two strings (pathname and basename) as if they >> are independent strings, but in reality, the latter is always >> pointing into a substring in the former. >> >> Clarify this relationship by expressing the latter as an offset into >> the former. >> >> Signed-off-by: Junio C Hamano > > This is a huge improvement in maintainability. My initial fix attempt > was to just xstrdup() the strings (knowing that the performance would be > horrible, but I was still investigating correctness issues at that > point). And of course I ran into this same issue as I tried to make a > copy of pathname. > > So even without the rest of the fix, this is definitely a good idea. :) match_{base,path}name and their exclude callers do the same thing. I guess I'm used to it and did not see the maintainability issue. Maybe we should do the same there too. -- Duy