Re: [PATCH 0/8] nd/resolve-ref v2
- From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
- Date
- Dec 6, 2011, 14:07 UTC
- 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>:
Show 9 quoted lines
> 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.
Show 5 quoted lines
>> 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.
Show 9 quoted lines
>> 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