[pacman-dev] Fwd: [BUG] chk_filedifference is buggy!!

[Nagy Gabor]

> Nagy, we've discussed this before in the past. *Please* do not just
> point out problems with the expectation that we will fix it. I know
> you are a very competent coder, and I'd appreciate you contributing
> solutions rather than inferring our code is shit. Saying things like
> "totally needless" does not start things off on the right foot with
> Aaron and I, who have really tried hard to clean up some questionable
> code. We do make mistakes.

Oh, I didn't want to say, that your code is shit (however: serious). Instead,
that function is quite tricky, and not easy to understand at all (that's why I
didn't provide a patch yet...: 1. I'm also quite tired/ill nowadays => I don't
trust on myself now; 2. I want to do a bigger rewrite of that part)

The word "totally needless" referred to the following: there is (should be) no
difference between empty and n-element list, so that pB==NULL check is not
needed <- the incorrect handling of pB==NULL was a special case of this BUG.
 
> diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
> index c093705..9ef5c9f 100644
> --- a/lib/libalpm/conflict.c
> +++ b/lib/libalpm/conflict.c
> @@ -279,6 +279,12 @@ static alpm_list_t
> *chk_filedifference(alpm_list_t *filesA, alpm_list_t *filesB)
>                         }
>           }
>         }
> +       /* ensure we have completely emptied pA */
> +       while(pA) {
> +               const char *strA = pA->data;
> +               ret = alpm_list_add(ret, strdup(strA));
> +               pA = pA->next;
> +       }
> 
>         return(ret);
>  }
> 

This seems OK, but the previous code filters out directories, so this is needed
here too (this was missed in the old pB==NULL handler part, too).

> This patch fixes the revised upgrade040 and 041 pactests, as well as
> upgrade011 as attached. It doesn't fix upgrade046.py. Any thoughts for
> that issue? I'm not familiar enough with the whole program flow there
> yet.
> 

Yes, for fixing upgrade046.py bigger efforts are needed (however, that is
probably rarer issue). The problem is, when you use --force, the
_alpm_db_find_fileconflicts is not called at all. And this file relocation
"stuff" (skip_remove) is computed in that function; so this computation is not
done at all with --force.

Bye


----------------------------------------------------
SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu
This mail sent through IMP: http://horde.org/imp/