Re: [PATCH v3 14/25] setup_discovered_git_dir(): plug memory leak

From: Stefan Beller <sbeller@google.com>
Date: May 2, 2017, 17:20 UTC
Message-ID: <CAGZ79kbsy1qh02WjX0nt6ci4r+5DVxJcPzwburBf0TP0qaEMTA@mail.gmail.com>
In-Reply-To: <ab131705c4383dc408860b516b1e18b0753b0cbf.1493740497.git.johannes.schindelin@gmx.de>

On Tue, May 2, 2017 at 9:02 AM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

Show 25 quoted lines

> The setup_explicit_git_dir() function does not take custody of the string
> passed as first parameter; we have to release it if we turned the value of
> git_dir into an absolute path.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  setup.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 0320a9ad14c..e3f7699a902 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -703,11 +703,16 @@ static const char *setup_discovered_git_dir(const char *gitdir,
>
>         /* --work-tree is set without --git-dir; use discovered one */
>         if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
> +               char *to_free = NULL;
> +               const char *ret;
> +
>                 if (offset != cwd->len && !is_absolute_path(gitdir))
> -                       gitdir = real_pathdup(gitdir, 1);
> +                       gitdir = to_free = real_pathdup(gitdir, 1);
>                 if (chdir(cwd->buf))
>                         die_errno("Could not come back to cwd");

As the original motivation was to shut up Coverity, this may not accomplish that goal, as in the path of taking the die_errno, we do not free `to_free`. But that is ok as the actual goal is to hav no memleaks in the good case. A memleak just before a die is no big deal.

> -               return setup_explicit_git_dir(gitdir, cwd, nongit_ok);
> +               ret = setup_explicit_git_dir(gitdir, cwd, nongit_ok);
> +               free(to_free);
> +               return ret;

And we free our pathdup from above, makes sense.

Thanks, Stefan