Re: [PATCH 06/26] get_mail_commit_oid(): avoid resource leak

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 26, 2017 at 1:19 PM, Johannes Schindelin
<johannes.schindelin@xxxxxx> wrote:
> When we fail to read, or parse, the file, we still want to close the file
> descriptor and release the strbuf.
>
> Reported via Coverity.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
>  builtin/am.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 805f56cec2f..01b700e5e74 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1359,15 +1359,16 @@ static int get_mail_commit_oid(struct object_id *commit_id, const char *mail)
>         struct strbuf sb = STRBUF_INIT;
>         FILE *fp = xfopen(mail, "r");
>         const char *x;
> +       int ret = 0;
>
>         if (strbuf_getline_lf(&sb, fp))
> -               return -1;
> +               ret = -1;
>
> -       if (!skip_prefix(sb.buf, "From ", &x))
> -               return -1;
> +       if (!ret && !skip_prefix(sb.buf, "From ", &x))
> +               ret = -1;
>
> -       if (get_oid_hex(x, commit_id) < 0)
> -               return -1;
> +       if (!ret && get_oid_hex(x, commit_id) < 0)
> +               ret = -1;
>

In similar cases of fixing mem leaks, we'd put a label here
and make excessive use of goto, instead of setting ret to -1.
As "ret" and the commands are short, this is visually just as appealing.

Thanks,
Stefan



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]