[pacman-dev] [PATCH] Fix alpm_list_add_sorted
Hi! Patch attached. Well, I dislike hacking the fundamental alpm_list.c, so I ask you to _doublecheck_ the patch before committing. Bye, ngaba ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
On Nov 13, 2007 11:01 AM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! Patch attached.
Could you explain exactly what this is fixing? The subject just says "fix" - is this something I missed with the tail pointer changes? Other than that, I think we're good. It looks... ok to me. And, for the record, I have to point out that I don't like comments like this, heh: + /* set add->next */ add->next = next;
On Nov 13, 2007 11:01 AM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! Patch attached.
Could you explain exactly what this is fixing? The subject just says "fix" - is this something I missed with the tail pointer changes? See the patch description: "The old code forgot about the case, when we insert the new node before the old list (set tail needed)" In other words, the old code just set add->prev = prev; in the beginning, but this breaks the (prev == NULL) case (without any further thinking: prev cannot be NULL now.)
Other than that, I think we're good. It looks... ok to me.
And, for the record, I have to point out that I don't like comments like this, heh: + /* set add->next */ add->next = next; Feel free to remove it. This was helpful to me; this is a comment of a _block_, it is something like /* step 1. */; because now a simple 'set add->next' can lead to complicated case-checking.
Bye, ngaba ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
On Nov 13, 2007 11:21 AM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On Nov 13, 2007 11:01 AM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! Patch attached.
Could you explain exactly what this is fixing? The subject just says "fix" - is this something I missed with the tail pointer changes?
Other than that, I think we're good. It looks... ok to me.
And, for the record, I have to point out that I don't like comments like this, heh: + /* set add->next */ add->next = next;
Aaron- I was going to let you run with this. -Dan
On Nov 13, 2007 11:21 AM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On Nov 13, 2007 11:01 AM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! Patch attached.
Could you explain exactly what this is fixing? The subject just says "fix" - is this something I missed with the tail pointer changes?
Other than that, I think we're good. It looks... ok to me.
And, for the record, I have to point out that I don't like comments like this, heh: + /* set add->next */ add->next = next;
Aaron- I was going to let you run with this.
Well, I can admit that my patch is quite hard-to-read. I wanted to keep the old style but the result is quite unreadable; probably a head-case, tail-case, middle-case would have been better... So I won't be aggrieved etc., if you rework it: this is teamwork. (I won't rewrite it, because it works(?), and code readability is not an argument for me.) Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
participants (3)
-
Aaron Griffin
-
Dan McGee
-
Nagy Gabor