Duy Nguyen <pclouds@xxxxxxxxx> writes:
> On Thu, Feb 20, 2014 at 6:26 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Is there a reason not to do just an equivalent of
>>
>> #define git_pathdup mkpathdup
>>
>> and be done with it? Am I missing something?
>
> They have a subtle difference: mkpathdup() calls cleanup_path() while
> git_pathdup() does not. Without auditing all call sites, we can't be
> sure this difference is insignificant. So I keep both functions
> separate for now.
Thanks; that is a very good explanation why #define'ing two to be
the same would not be a workable solution to the ownership issue I
raised.
> char *git_pathdup(const char *fmt, ...)
> {
> - char path[PATH_MAX], *ret;
> + struct strbuf *path = get_pathname();
> va_list args;
> va_start(args, fmt);
> - ret = vsnpath(path, sizeof(path), fmt, args);
> + vsnpath(path, fmt, args);
> va_end(args);
> - return xstrdup(ret);
> + return strbuf_detach(path, NULL);
> }
This feels somewhat wrong.
This function is for callers who are willing to take ownership of
the path buffer and promise to free the returned buffer when they
are done, because you are returning strbuf_detach()'ed piece of
memory, giving the ownership away.
The whole point of using get_pathname() is to allow callers not to
care about allocation issues on the paths they scribble on during
their short-and-simple codepaths that do not have too many uses of
similar temporary path buffers. Why borrow from that round-robin
pool (which may now cause some codepaths to overflow the number of
such active temporary path buffers---have they been all audited)?
--
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