FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 01-01-2008, 03:25 PM
Chantry Xavier
 
Default new upgade042 pactest + bufix in chk_filedifference.

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@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;
diff --git a/pactest/tests/upgrade042.py b/pactest/tests/upgrade042.py
new file mode 100644
index 0000000..d6140d4
--- /dev/null
+++ b/pactest/tests/upgrade042.py
@@ -0,0 +1,28 @@
+self.description = "Backup file relocation"
+
+lp1 = pmpkg("bash")
+lp1.files = ["etc/profile*"]
+lp1.backup = ["etc/profile"]
+self.addpkg2db("local", lp1)
+
+p1 = pmpkg("bash", "1.0-2")
+self.addpkg(p1)
+
+lp2 = pmpkg("filesystem")
+self.addpkg2db("local", lp2)
+
+p2 = pmpkg("filesystem", "1.0-2")
+p2.files = ["etc/profile**"]
+p2.backup = ["etc/profile"]
+p2.depends = [ "bash" ]
+self.addpkg(p2)
+
+self.args = "-U %s" % " ".join([p.filename() for p in p1, p2])
+
+self.filesystem = ["etc/profile"]
+
+self.addrule("PKG_VERSION=bash|1.0-2")
+self.addrule("PKG_VERSION=filesystem|1.0-2")
+self.addrule("!FILE_PACSAVE=etc/profile")
+self.addrule("FILE_PACNEW=etc/profile")
+self.addrule("FILE_EXIST=etc/profile")
--
1.5.3.7


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 01-02-2008, 12:56 AM
"Dan McGee"
 
Default new upgade042 pactest + bufix in chk_filedifference.

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

On Jan 1, 2008 10:25 AM, Chantry Xavier <shiningxc@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@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

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 01-02-2008, 02:38 AM
"Scott Horowitz"
 
Default new upgade042 pactest + bufix in chk_filedifference.

On Jan 1, 2008 6:56 PM, Dan McGee <dpmcgee@gmail.com> wrote:
> 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.

Weird. The lesson learned in my eyes is to stick with python ;-)

Scott

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 01-02-2008, 06:55 AM
Xavier
 
Default new upgade042 pactest + bufix in chk_filedifference.

Dan McGee wrote:
>
> 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.
>

Oh sorry, I totally overlooked that indeed..
If only it had segfaulted in my test case, I would have caught it
immediatly.
That's bad, I wanted to fix a minor bug and caused a much bigger one

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 01-02-2008, 12:07 PM
"Dan McGee"
 
Default new upgade042 pactest + bufix in chk_filedifference.

On Jan 2, 2008 1:55 AM, Xavier <shiningxc@gmail.com> wrote:
> Dan McGee wrote:
> >
> > 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.
> >
>
> Oh sorry, I totally overlooked that indeed..
> If only it had segfaulted in my test case, I would have caught it
> immediatly.
> That's bad, I wanted to fix a minor bug and caused a much bigger one

Well your pactest did end up causing the segfault, but not until
*after* I made those completely unrelated changes. Memory allocation
can do weird things sometimes.

-Dan

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 

Thread Tools




All times are GMT. The time now is 03:15 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org