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. > Convert many resolve_ref() calls to read_ref*() and ref_exists() > Rename resolve_ref() to resolve_ref_unsafe() I like the general direction of the series (and especially patch 1/8). As Junio nicely explains at [1], it is too tempting to keep and pass around the canonical representation of a refname returned by resolve_ref() without remembering to copy it. > 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? > cmd_merge: convert to single exit point > Use resolve_ref() instead of resolve_ref_unsafe() The print_summary() change introduces a leak. [...] > Guard memory overwriting in resolve_ref_unsafe's static buffer > 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. Ciao, Jonathan [1] http://thread.gmane.org/gmane.comp.version-control.git/185446/focus=185519 -- 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