On Mon, Mar 28, 2016 at 11:56:10PM +0800, Hui Yiqun wrote:
> According to strbuf.h, strbuf_detach is the sole supported method
> to unwrap a memory buffer from its strbuf shell.
>
> So we should not return the pointer of strbuf.buf directly.
>
> What's more, some memory leakages are solved.
There is something else going on here, which makes this case different
than some others. Note that the function returns a const string:
> diff --git a/path.c b/path.c
> index 969b494..b07e5a7 100644
> --- a/path.c
> +++ b/path.c
> @@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
By convention, that means that the result is not owned by the caller,
and should not be freed. We implement that by:
> {
> static struct strbuf validated_path = STRBUF_INIT;
> static struct strbuf used_path = STRBUF_INIT;
...using static variables which persist after the call returns. So this
function retains ownership of the memory, and it remains valid until the
next call to enter_repo().
There's no leak when we return NULL, because the function retains
control of the memory (though it will hold onto it until the end of the
program if nobody calls enter_repo() again). And thus we shouldn't use
strbuf_detach(), which loses that reference to the memory (and since the
callers don't take ownership, it actually _creates_ a leak).
We could release the memory when returning, but I don't think it's a big
deal to do so.
-Peff
--
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