[pacman-dev] [PATCH] new upgade042 pactest + bufix in chk_filedifference.

[Dan McGee]

Whoa, you caused me a lot of pain with this one. See below.

On Jan 1, 2008 10:25 AM, Chantry Xavier <shiningxc at gmail.com> wrote:
> This adds a pactest for the relocation of a config file between two packages
> (case of etc/profile moving from bash to filesystem).
> While running this pactest, I found out that chk_filedifference didn't work
> correctly whith an empty list as second argument. So that's fixed now.
>
> Ref: http://www.archlinux.org/pipermail/pacman-dev/2007-December/010610.html
>
> Signed-off-by: Chantry Xavier <shiningxc at gmail.com>
> ---
>  lib/libalpm/conflict.c      |    4 ++++
>  pactest/tests/upgrade042.py |   28 ++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+), 0 deletions(-)
>  create mode 100644 pactest/tests/upgrade042.py
>
> diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
> index 91f4360..c1aac4d 100644
> --- a/lib/libalpm/conflict.c
> +++ b/lib/libalpm/conflict.c
> @@ -251,6 +251,10 @@ static alpm_list_t *chk_filedifference(alpm_list_t *filesA, alpm_list_t *filesB)
>         alpm_list_t *ret = NULL;
>         alpm_list_t *pA = filesA, *pB = filesB;
>
> +       if(pB == NULL) {
> +               return(pA);
> +       }
> +
>         while(pA && pB) {
>                 const char *strA = pA->data;
>                 const char *strB = pB->data;

Note that chk_filedifference always returned a NEW list, thus the
return value could always be freed by the calling function. Thus, your
change here gave me a big fat segfault because you didn't dupe the
list first. I've fixed this locally and replaced the return with a
alpm_list_strdup(pA) call.

This segfault didn't get exposed until I made a *completely* unrelated
change (removing a gettext call around a message that doesn't need to
be gettexted), so it took some time to track down.

Lesson to be learned here: know whether your function allocates new
objects or not, and ensure ALL return paths do it the same way. Then
be sure to check that the calling function does a free if necessary.

-Dan