From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Tue, 06 Dec 2011 14:07:09 GMT
Subject: Re: [PATCH 0/8] nd/resolve-ref v2
Message-ID: <CACsJy8A6KqjOjMQQXEdHWeAUqB5np-_myXdQzQe1+oMfXFzv7Q@mail.gmail.com>
In-Reply-To: <20111117103924.GA5277@elie.hsd1.il.comcast.net>

2011/11/17 Jonathan Nieder <jrnieder@gmail.com>:
> Hi,
>
> Nguyễn Thái Ngọc Duy wrote:
>
>> Nguyễn Thái Ngọc Duy (8):
>
> Here are some comments from a quick glance over the series, to avoid
> too much noise that would distract from reports about the upcoming
> release.

Sorry for the late reply. Somehow I managed to miss your mail.

>>   Re-add resolve_ref() that always returns an allocated buffer
>
> Makes me nervous, since it would introduce memory leaks if some other
> patch in flight calls resolve_ref().  Why not call it ref_resolve() or
> something?

Yeah, I made the same mistake before and made it again.

>>   Enable GIT_DEBUG_MEMCHECK on git_pathname()
>
> __VA_ARGS__ was introduced in C99.  I suspect some compilers that
> currently can compile git don't support it.  But if you need to use
> it, that wouldn't rule out doing so in some corner guarded with an
> #ifdef.
>
> Looks pleasant overall.  I look forward to looking more closely at
> this later.

This patch is bonus anyway and I think I'll drop it. Keeping a bunch
of ifdefs looks really ugly. I think having git_pathname()'s static
buffer per callsite file would be a good thing to do instead.
-- 
Duy