Re: [PATCH 0/8] nd/resolve-ref v2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2011/11/17 Jonathan Nieder <jrnieder@xxxxxxxxx>:
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]