Re: [PATCH v11 0/8] submodule inline diff format

From: Jacob Keller <redacted>

On Fri, Aug 26, 2016 at 1:04 PM, Jeff King [off-list ref] wrote:

quoted

On Fri, Aug 26, 2016 at 07:58:07PM +0000, Keller, Jacob E wrote:

quoted

quoted

quoted

 char *git_pathdup_submodule(const char *path, const char *fmt,
...)
 {
+       int err;
        va_list args;
        struct strbuf buf = STRBUF_INIT;
        va_start(args, fmt);
-       do_submodule_path(&buf, path, fmt, args);
+       err = do_submodule_path(&buf, path, fmt, args);
        va_end(args);
+       if (err)

Here we need a strbuf_release(&buf) to avoid a memory leak?

No, cause we "strbuf_detach" after this to return the buffer? Or is
that not safe?

That code path is OK. I think the question is whether you need to
release the buffer in the "err" case where you return NULL and don't hit
the strbuf_detach.

IOW, does do_submodule_path() promise that when it returns an error,
"buf" has been left uninitialized? Some of our strbuf functions do, but
I do not know offhand about do_submodule_path().

-Peff

We probably should release for the error case. I'll do that. I don't
believe do_submodule_path ensures that the passed in argument is
guaranteed to not be initialized or used.

Thanks,
Jake

Here's the squash for this fix.

Thanks,
Jake

---------->8

From Jacob Keller [off-list ref]
From 9cf89634e6f2b0f3f90f43a553f55eb57bb2f662 Mon Sep 17 00:00:00 2001
From: Jacob Keller <redacted>
Date: Fri, 26 Aug 2016 16:06:54 -0700
Subject: [PATCH] squash! allow do_submodule_path to work even if submodule
 isn't checked out

Add a missing strbuf_release() when returning during the error flow of
git_pathdup_submodule().

Signed-off-by: Jacob Keller <redacted>
---
 path.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/path.c b/path.c
index e9369f75319d..b8515973252c 100644
--- a/path.c
+++ b/path.c

@@ -525,8 +525,10 @@ char *git_pathdup_submodule(const char *path, const char *fmt, ...)
 	va_start(args, fmt);
 	err = do_submodule_path(&buf, path, fmt, args);
 	va_end(args);
-	if (err)
+	if (err) {
+		strbuf_release(&buf);
 		return NULL;
+	}
 	return strbuf_detach(&buf, NULL);
 }
 

-- 
2.10.0.rc0.259.g83512d9