[pacman-dev] [PATCH 2/3] lib/libalpm/deps.c: Fix memory leak in _alpm_dep_dup() in case of error.

[Dan McGee]

On Sat, Aug 27, 2011 at 4:03 PM, Diogo Sousa <diogogsousa at gmail.com> wrote:
> Signed-off-by: Diogo Sousa <diogogsousa at gmail.com>
I am not a fan of labels like 'error1', 'error2', this will result in
spaghetti disasters down the road.

Given that free(NULL) is a no-op by standard, you can use one error:
label here and just unwind in order- no need for multiple labels and
we can afford the no-ops given this is an extreme error case (and we
are likely about to eat it and die anyway).

Same thing applies for the other, similar patch.

> ---
>  lib/libalpm/deps.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
> index 47b7637..4c05191 100644
> --- a/lib/libalpm/deps.c
> +++ b/lib/libalpm/deps.c
> @@ -455,12 +455,19 @@ alpm_depend_t *_alpm_dep_dup(const alpm_depend_t *dep)
>        alpm_depend_t *newdep;
>        CALLOC(newdep, 1, sizeof(alpm_depend_t), return NULL);
>
> -       STRDUP(newdep->name, dep->name, return NULL);
> +       STRDUP(newdep->name, dep->name, goto error1);
>        newdep->name_hash = dep->name_hash;
> -       STRDUP(newdep->version, dep->version, return NULL);
> +       STRDUP(newdep->version, dep->version, goto error2);
>        newdep->mod = dep->mod;
>
>        return newdep;
> +
> +error2:
> +       free(newdep->name);
> +error1:
> +       free(newdep);
> +
> +       return NULL;
>  }
>
>  /* These parameters are messy. We check if this package, given a list of
> --
> 1.7.6.1