On 17/07/12 17:19, Lukas Fleischer wrote:
> On Tue, Jul 17, 2012 at 11:50:26AM +1000, Allan McRae wrote:
>> Detect a conflict between a file/symlink in one package and a directory
>> in another when both are being installed at once.
>>
>> A side effect is the creation on conflicts between a direcotry symlink
>> and a real directory (e.g lib -> usr/lib in pkg1 and /lib in pkg2).
>> Given we can not guarantee pkg1 is installed before pkg2, this is a
>> genuine conflict.
>>
>> Signed-off-by: Allan McRae <[email protected]>
>> ---
>> lib/libalpm/conflict.c | 48
>> +++++++++++++++++++++++++-----------
>> test/pacman/tests/fileconflict002.py | 2 --
>> test/pacman/tests/fileconflict015.py | 2 --
>> 3 files changed, 34 insertions(+), 18 deletions(-)
>>
>> diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
>> index f3b269f..041ba6f 100644
>> --- a/lib/libalpm/conflict.c
>> +++ b/lib/libalpm/conflict.c
>> @@ -274,28 +274,48 @@ static alpm_list_t
>> *filelist_intersection(alpm_filelist_t *filesA,
>> size_t ctrA = 0, ctrB = 0;
>>
>> while(ctrA < filesA->count && ctrB < filesB->count) {
>> + int cmp, isdirA, isdirB;
>> +
>> alpm_file_t *fileA = filesA->files + ctrA;
>> alpm_file_t *fileB = filesB->files + ctrB;
>> - const char *strA = fileA->name;
>> - const char *strB = fileB->name;
>> - /* skip directories, we don't care about them */
>> + char *strA = strdup(fileA->name);
>> + char *strB = strdup(fileB->name);
>> +
>> + isdirA = 0;
>> if(strA[strlen(strA)-1] == '/') {
>> + isdirA = 1;
>> + strA[strlen(strA)-1] = '\0';
>> + }
>> +
>> + isdirB = 0;
>> + if(strB[strlen(strB)-1] == '/') {
>> + isdirB = 1;
>> + strB[strlen(strB)-1] = '\0';
>> + }
>> +
>> + cmp = strcmp(strA, strB);
>
> Allocating extra memory on the heap just to chop off one character
> before comparing doesn't look like the right thing to do... The problem
> is that we probably cannot leave the strings as-is without writing our
> custom strcmp() implementation or doing some strncmp() magic (I didn't
> look into strncmp() in detail), though.
>
> Even if you want to keep it simple and use strdup(), you could
> initialize "strA" and "strB" with pointers to the original strings and
> move the strdup() into the two if-branches. Note that in this case, you
> will also add one additional check to the free() statements but this
> will remove the need for heap and copy operations for the majority of
> files, won't it?
Changed so strndup is only used when the file is a directory.
> Also, why do we use mixedCase for variables here (same applies to the
> patch you submitted before this one)? :)
Because that was the way the variables were named beforehand...
>> + if(cmp < 0) {
>> ctrA++;
>> - } else if(strB[strlen(strB)-1] == '/') {
>> + } else if(cmp > 0) {
>> ctrB++;
>> } else {
>> - int cmp = strcmp(strA, strB);
>> - if(cmp < 0) {
>> - ctrA++;
>> - } else if(cmp > 0) {
>> - ctrB++;
>> - } else {
>> - /* item in both, qualifies as an intersect */
>> + /* TODO: this creates conflicts between a symlink to a
>> directory in
>> + * one package and a real directory in the other. For
>> example,
>> + * lib -> /usr/lib in pkg1 and /lib in pkg2. This
>> would be allowed
>> + * when installing one package at a time _provided_
>> pkg1 is installed
>> + * first. This will need adjusted if the order of
>> package install can
>> + * be guaranteed to install the symlink first */
>> +
>> + /* when not directories, item in both qualifies as an
>> intersect */
>> + if(! (isdirA && isdirB)) {
>> ret = alpm_list_add(ret, fileA);
>> - ctrA++;
>> - ctrB++;
>> }
>> - }
>> + ctrA++;
>> + ctrB++;
>> + }
>> +
>> + free(strA);
>> + free(strB);
>> }
>>
>> return ret;
>> diff --git a/test/pacman/tests/fileconflict002.py
>> b/test/pacman/tests/fileconflict002.py
>> index e107cd2..1e6113c 100644
>> --- a/test/pacman/tests/fileconflict002.py
>> +++ b/test/pacman/tests/fileconflict002.py
>> @@ -19,5 +19,3 @@
>> self.addrule("!PKG_EXIST=pkg1")
>> self.addrule("!PKG_EXIST=pkg2")
>> self.addrule("!FILE_EXIST=dir/realdir/file")
>> -
>> -self.expectfailure = True
>> diff --git a/test/pacman/tests/fileconflict015.py
>> b/test/pacman/tests/fileconflict015.py
>> index 78634d7..3c80bbc 100644
>> --- a/test/pacman/tests/fileconflict015.py
>> +++ b/test/pacman/tests/fileconflict015.py
>> @@ -13,5 +13,3 @@
>> self.addrule("PACMAN_RETCODE=1")
>> self.addrule("!PKG_EXIST=pkg1")
>> self.addrule("!PKG_EXIST=pkg2")
>> -
>> -self.expectfailure = True
>> --
>> 1.7.11.2
>
>
>
