Stefan Beller <sbeller@xxxxxxxxxx> writes:
> On Mon, Jan 2, 2017 at 8:22 AM, Johannes Schindelin
> <johannes.schindelin@xxxxxx> wrote:
>> Technically, it is correct that git_exec_path() returns a possibly
>> malloc()ed string. Practically, it is *sometimes* not malloc()ed. So
>> let's just use a static variable to make it a singleton. That'll shut
>> Coverity up, hopefully.
>
> I picked up this patch and applied it to the coverity branch
> that I maintain at github/stefanbeller/git.
>
> I'd love to see this patch upstream as it reduces my maintenance
> burden of the coverity branch by a patch.
So with the above, are you saying "Dscho said 'hopefully', and I
confirm that this change does squelch misdiagnosis by Coverity"?
> If this patch is only to appease coverity (as the commit message eludes
> to) I think this may be a bad idea for upstream. If this patch fixes an
> actual problem, then the commit message needs to spell that out.
That is true, and I see Peff pointed out another possible issue
around getenv(), but I think from the "one step at a time" point of
view, it is an improvement to call system_path() just once and cache
it in "static char *".
How about explaining it like this then?
(only the log message has been corrected; diff is from the original).
commit c9bb5d101ca657fa466afa8c4368c43ea7b7aca8
Author: Johannes Schindelin <johannes.schindelin@xxxxxx>
Date: Mon Jan 2 17:22:33 2017 +0100
git_exec_path: avoid Coverity warning about unfree()d result
Technically, it is correct that git_exec_path() returns a possibly
malloc()ed string returned from system_path(), and it is sometimes
not allocated. Cache the result in a static variable and make sure
that we call system_path() only once, which plugs a potential leak.
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
diff --git a/exec_cmd.c b/exec_cmd.c
index 9d5703a157..eae56fefba 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -69,6 +69,7 @@ void git_set_argv_exec_path(const char *exec_path)
const char *git_exec_path(void)
{
const char *env;
+ static char *system_exec_path;
if (argv_exec_path)
return argv_exec_path;
@@ -78,7 +79,9 @@ const char *git_exec_path(void)
return env;
}
- return system_path(GIT_EXEC_PATH);
+ if (!system_exec_path)
+ system_exec_path = system_path(GIT_EXEC_PATH);
+ return system_exec_path;
}
static void add_path(struct strbuf *out, const char *path)