Hi! It is not a good idea to compare strings with memcmp: (abc'\0'd should be equivalent with abc'\0') Bye, ngaba
On 10/22/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! It is not a good idea to compare strings with memcmp: (abc'\0'd should be equivalent with abc'\0')
Please either send a patch or file a bug report. Mailing lists are not the appropriate place for this.
Hi! Patch attached in git format ;-) The patch was created to the current git repo, so it conflicts with my pmconflict_t patch, but I can rework one of them on request. Bye, good-boy-ngaba
On 10/24/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! Patch attached in git format ;-) The patch was created to the current git repo, so it conflicts with my pmconflict_t patch, but I can rework one of them on request. Bye, good-boy-ngaba
Pushed to my working branch, thanks! http://code.toofishes.net/gitweb.cgi?p=pacman.git;a=commitdiff;h=89ac8aa9c45... -Dan P.S. Next time feel free to inline it, but we'll take it one step at a time. :)
On Wed, Oct 24, 2007 at 10:58:34PM +0200, Nagy Gabor wrote:
--- lib/libalpm/deps.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index b7e49be..69c675c 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -98,8 +98,11 @@ int _alpm_depmiss_isin(pmdepmissing_t *needle, alpm_list_t *haystack)
for(i = haystack; i; i = i->next) { pmdepmissing_t *miss = i->data; - if(!memcmp(needle, miss, sizeof(pmdepmissing_t)) - && !memcmp(&needle->depend, &miss->depend, sizeof(pmdepend_t))) {
Could someone explain what these two lines are supposed to do, because it doesn't make any sense to me. The second line looks redundant. Also I fail to see any real cases where this function would return true. Though it's easy to trigger it with a simple pactest, where a package has the same dependency listed twice. That doesn't make any sense but well... This check probably doesn't hurt.
On 10/24/07, Xavier <shiningxc@gmail.com> wrote:
On Wed, Oct 24, 2007 at 10:58:34PM +0200, Nagy Gabor wrote:
--- lib/libalpm/deps.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index b7e49be..69c675c 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -98,8 +98,11 @@ int _alpm_depmiss_isin(pmdepmissing_t *needle, alpm_list_t *haystack)
for(i = haystack; i; i = i->next) { pmdepmissing_t *miss = i->data; - if(!memcmp(needle, miss, sizeof(pmdepmissing_t)) - && !memcmp(&needle->depend, &miss->depend, sizeof(pmdepend_t))) {
Could someone explain what these two lines are supposed to do, because it doesn't make any sense to me. The second line looks redundant.
It's comparing the entire structure. When you do with with a structure that has a pointer, it's comparing, say 0x8239482308 to 0x8239482308, which isn't always what we want. The second line compares the actual structures POINTED TO
On Wed, Oct 24, 2007 at 05:23:01PM -0500, Aaron Griffin wrote:
On 10/24/07, Xavier <shiningxc@gmail.com> wrote:
On Wed, Oct 24, 2007 at 10:58:34PM +0200, Nagy Gabor wrote:
--- lib/libalpm/deps.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index b7e49be..69c675c 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -98,8 +98,11 @@ int _alpm_depmiss_isin(pmdepmissing_t *needle, alpm_list_t *haystack)
for(i = haystack; i; i = i->next) { pmdepmissing_t *miss = i->data; - if(!memcmp(needle, miss, sizeof(pmdepmissing_t)) - && !memcmp(&needle->depend, &miss->depend, sizeof(pmdepend_t))) {
Could someone explain what these two lines are supposed to do, because it doesn't make any sense to me. The second line looks redundant.
It's comparing the entire structure. When you do with with a structure that has a pointer, it's comparing, say 0x8239482308 to 0x8239482308, which isn't always what we want. The second line compares the actual structures POINTED TO
But it's not a pointer, is it? /* Dependency */ struct __pmdepend_t { pmdepmod_t mod; char name[PKG_NAME_LEN]; char version[PKG_VERSION_LEN]; }; /* Missing dependency */ struct __pmdepmissing_t { char target[PKG_NAME_LEN]; pmdeptype_t type; pmdepend_t depend; }; If it was a pointer, it wouldn't make sense either, because the first memcmp would already have compared the address of needle->depend and miss->depend.
On 10/24/07, Xavier <shiningxc@gmail.com> wrote:
On Wed, Oct 24, 2007 at 05:23:01PM -0500, Aaron Griffin wrote:
On 10/24/07, Xavier <shiningxc@gmail.com> wrote:
On Wed, Oct 24, 2007 at 10:58:34PM +0200, Nagy Gabor wrote:
--- lib/libalpm/deps.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index b7e49be..69c675c 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -98,8 +98,11 @@ int _alpm_depmiss_isin(pmdepmissing_t *needle, alpm_list_t *haystack)
for(i = haystack; i; i = i->next) { pmdepmissing_t *miss = i->data; - if(!memcmp(needle, miss, sizeof(pmdepmissing_t)) - && !memcmp(&needle->depend, &miss->depend, sizeof(pmdepend_t))) {
Could someone explain what these two lines are supposed to do, because it doesn't make any sense to me. The second line looks redundant.
It's comparing the entire structure. When you do with with a structure that has a pointer, it's comparing, say 0x8239482308 to 0x8239482308, which isn't always what we want. The second line compares the actual structures POINTED TO
But it's not a pointer, is it?
/* Dependency */ struct __pmdepend_t { pmdepmod_t mod; char name[PKG_NAME_LEN]; char version[PKG_VERSION_LEN]; };
/* Missing dependency */ struct __pmdepmissing_t { char target[PKG_NAME_LEN]; pmdeptype_t type; pmdepend_t depend; };
If it was a pointer, it wouldn't make sense either, because the first memcmp would already have compared the address of needle->depend and miss->depend.
Well, I'll be! If it's not a pointer, than it IS redundant. Were it a pointer though, the first case would fail and the second call would be made...
On Wed, Oct 24, 2007 at 05:38:45PM -0500, Aaron Griffin wrote:
Well, I'll be! If it's not a pointer, than it IS redundant. Were it a pointer though, the first case would fail and the second call would be made...
I don't think so, it's a &&. Anyway, forget it, I see that Dan applied Nagy's patch, so it's alright. I was just curious.
Could someone explain what these two lines are supposed to do, because it doesn't make any sense to me. The second line looks redundant. Yes. This was my first impression, too ;-)
participants (4)
-
Aaron Griffin
-
Dan McGee
-
Nagy Gabor
-
Xavier