Stefan Beller <[email protected]> writes: > On Mon, Jan 2, 2017 at 8:22 AM, Johannes Schindelin > <[email protected]> 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 <[email protected]> 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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]> 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)
