Re: [PATCH v3] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage

From: 惠轶群 <huiyiqun@gmail.com>
Date: Mar 28, 2016, 15:59 UTC
Message-ID: <CAKqreuzoQmkUwyYiAt_GhoHyaCz=WoNshOkhH=D3orNZEG2ayQ@mail.gmail.com>
In-Reply-To: <1459180638-6034-1-git-send-email-huiyiqun@gmail.com>

Sorry, I sent the patch repeatedly to fix a wrongly indent with space.

2016-03-28 23:57 GMT+08:00 Hui Yiqun <huiyiqun@gmail.com>:

Show 72 quoted lines

> 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.
> ---
>  path.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/path.c b/path.c
> index 969b494..9801617 100644
> --- a/path.c
> +++ b/path.c
> @@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
>  {
>         static struct strbuf validated_path = STRBUF_INIT;
>         static struct strbuf used_path = STRBUF_INIT;
> +       char * dbuf = NULL;
>
>         if (!path)
>                 return NULL;
> @@ -654,7 +655,7 @@ const char *enter_repo(const char *path, int strict)
>                 if (used_path.buf[0] == '~') {
>                         char *newpath = expand_user_path(used_path.buf);
>                         if (!newpath)
> -                               return NULL;
> +                               goto return_null;
>                         strbuf_attach(&used_path, newpath, strlen(newpath),
>                                       strlen(newpath));
>                 }
> @@ -671,22 +672,22 @@ const char *enter_repo(const char *path, int strict)
>                         strbuf_setlen(&used_path, baselen);
>                 }
>                 if (!suffix[i])
> -                       return NULL;
> +                       goto return_null;
>                 gitfile = read_gitfile(used_path.buf);
>                 if (gitfile) {
>                         strbuf_reset(&used_path);
>                         strbuf_addstr(&used_path, gitfile);
>                 }
>                 if (chdir(used_path.buf))
> -                       return NULL;
> -               path = validated_path.buf;
> +                       goto return_null;
> +               path = dbuf = strbuf_detach(&validated_path, NULL);
>         }
>         else {
>                 const char *gitfile = read_gitfile(path);
>                 if (gitfile)
>                         path = gitfile;
>                 if (chdir(path))
> -                       return NULL;
> +                       goto return_null;
>         }
>
>         if (is_git_directory(".")) {
> @@ -695,6 +696,10 @@ const char *enter_repo(const char *path, int strict)
>                 return path;
>         }
>
> +return_null:
> +       free(dbuf);
> +       strbuf_release(&used_path);
> +       strbuf_release(&validated_path);
>         return NULL;
>  }
>
> --
> 2.7.4
>