[pacman-dev] alpm_list_count gives 1 on an list created by alpm_list_new
hi. i am just using my free time to code with alpm and alpm_list. nothing to be published yet (or ever ?) today i just noticed that a list created with alpm_list_new has a number of element of 1 as alpm_list_count reports it: #include <stdlib.h> #include <stdio.h> #include <alpm_list.h> int main(void) { alpm_list_t *l; l = alpm_list_new(); printf("count=%d\n",alpm_list_count(l)); return(0); } $ ./test count=1 shining wanted me to post here about that. that's done !
hi.
i am just using my free time to code with alpm and alpm_list. nothing to be published yet (or ever ?)
today i just noticed that a list created with alpm_list_new has a number of element of 1 as alpm_list_count reports it: #include <stdlib.h> #include <stdio.h>
#include <alpm_list.h>
int main(void) { alpm_list_t *l; l = alpm_list_new(); printf("count=%d\n",alpm_list_count(l)); return(0); }
$ ./test count=1
shining wanted me to post here about that. that's done !
Yes, this is not nice, but needed (we doesn't have alpm_list_node_t or alpm_list_head_t struct): list == NULL is not allowed [otherwise list->data or list->next would lead to a segfault.] Workaround: alpm_list_count could count nodes with not-NULL data pointers only. Btw, this also indicates a more serious bug: as I see, if you remove the only element from a 1-element list, alpm_list_remove() will return with NULL, with is not a valid list pointer... Bye
Yes, this is not nice, but needed (we doesn't have alpm_list_node_t or alpm_list_head_t struct): list == NULL is not allowed [otherwise list->data or list->next would lead to a segfault.] Workaround: alpm_list_count could count nodes with not-NULL data pointers only.
Btw, this also indicates a more serious bug: as I see, if you remove the only element from a 1-element list, alpm_list_remove() will return with NULL, with is not a valid list pointer...
Bye
Correction of myself: since we usually use 'for(i = list; i; i = i->next)', i==NULL is acceptable. So IMHO list = alpm_list_new() <=> list = NULL; would be better; and rename the current alpm_list_new() to _internal_ alpm_list_node_new() [used by alpm_list_add ...]. Then the hackish line 119 could also disappear from alpm_list.c Bye
On Sat, Feb 23, 2008 at 08:36:24PM +0100, Nagy Gabor wrote:
Yes, this is not nice, but needed (we doesn't have alpm_list_node_t or alpm_list_head_t struct): list == NULL is not allowed [otherwise list->data or list->next would lead to a segfault.] Workaround: alpm_list_count could count nodes with not-NULL data pointers only.
Btw, this also indicates a more serious bug: as I see, if you remove the only element from a 1-element list, alpm_list_remove() will return with NULL, with is not a valid list pointer...
Bye
Correction of myself: since we usually use 'for(i = list; i; i = i->next)', i==NULL is acceptable. So IMHO list = alpm_list_new() <=> list = NULL; would be better; and rename the current alpm_list_new() to _internal_ alpm_list_node_new() [used by alpm_list_add ...].
Exactly what I was thinking, but wasn't sure. Thanks.
Then the hackish line 119 could also disappear from alpm_list.c
Hmm, why does renaming a function allows us to do that? Any way you can provide a patch for this? it would be appreciated ;)
From 6d3d84fa38a6d67f441c2b76177a66da72692eec Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Sat, 23 Feb 2008 22:31:20 +0100 Subject: [PATCH] alpm_list.c clean-up *this patch introduces 'list == NULL' convention for empty list *other small straightforward fixes in alpm_list.c Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> --- lib/libalpm/alpm_list.c | 129 ++++++++++++++++++++++++++--------------------- 1 files changed, 71 insertions(+), 58 deletions(-) diff --git a/lib/libalpm/alpm_list.c b/lib/libalpm/alpm_list.c index 57c8376..695dd19 100644 --- a/lib/libalpm/alpm_list.c +++ b/lib/libalpm/alpm_list.c @@ -40,20 +40,23 @@ /* Allocation */ /** - * @brief Allocate a new alpm_list_t. - * - * @return a new alpm_list_t item, or NULL on failure + * @brief Create an empty list. */ alpm_list_t SYMEXPORT *alpm_list_new() { + return(NULL); +} + +/** + * @brief Allocate a new node for alpm_list_t (with empty ->data) + * + * @return a new node item, or NULL on failure + */ +static alpm_list_t *alpm_list_node_new() +{ alpm_list_t *list = NULL; - list = malloc(sizeof(alpm_list_t)); - if(list) { - list->data = NULL; - list->prev = list; /* maintain a back reference to the tail pointer */ - list->next = NULL; - } + list = calloc(1, sizeof(alpm_list_t)); return(list); } @@ -107,30 +110,26 @@ alpm_list_t SYMEXPORT *alpm_list_add(alpm_list_t *list, void *data) { alpm_list_t *ptr, *lp; - ptr = list; + ptr = alpm_list_node_new(); if(ptr == NULL) { - ptr = alpm_list_new(); - if(ptr == NULL) { - return(NULL); - } + return(list); } - lp = alpm_list_last(ptr); - if(lp == ptr && lp->data == NULL) { - /* nada */ - } else { - lp->next = alpm_list_new(); - if(lp->next == NULL) { - return(NULL); - } - lp->next->prev = lp; - lp = lp->next; - list->prev = lp; + ptr->data = data; + ptr->next = NULL; + + /* Special case: the input list is empty */ + if(list == NULL) { + ptr->prev = ptr; + return(ptr); } - lp->data = data; + lp = alpm_list_last(list); + lp->next = ptr; + ptr->prev = lp; + list->prev = ptr; - return(ptr); + return(list); } /** @@ -144,12 +143,15 @@ alpm_list_t SYMEXPORT *alpm_list_add(alpm_list_t *list, void *data) */ alpm_list_t SYMEXPORT *alpm_list_add_sorted(alpm_list_t *list, void *data, alpm_list_fn_cmp fn) { - if(!fn) { - return alpm_list_add(list, data); + if(!fn || !list) { + return(alpm_list_add(list, data)); } else { alpm_list_t *add = NULL, *prev = NULL, *next = list; - add = alpm_list_new(); + add = alpm_list_node_new(); + if(add == NULL) { + return(list); + } add->data = data; /* Find insertion point. */ @@ -159,26 +161,25 @@ alpm_list_t SYMEXPORT *alpm_list_add_sorted(alpm_list_t *list, void *data, alpm_ next = next->next; } - /* Insert node before insertion point. */ - add->prev = prev; - add->next = next; - - if(next != NULL) { - next->prev = add; /* Not at end. */ - } - - if(prev != NULL) { - prev->next = add; /* In middle. */ - } else { - list = add; /* At beginning, or new list */ - } - - if(next == NULL) { - /* At end, adjust tail pointer on head node */ + /* Insert the add node to the list */ + if(prev == NULL) { /* special case: we insert add as the first element */ + add->prev = list->prev; /* list != NULL */ + add->next = list; + list->prev = add; + return(add); + } else if(next == NULL) { /* another special case: add last element */ + add->prev = prev; + add->next = NULL; + prev->next = add; list->prev = add; + return(list); + } else { + add->prev = prev; + add->next = next; + next->prev = add; + prev->next = add; + return(list); } - - return(list); } } @@ -198,10 +199,10 @@ alpm_list_t SYMEXPORT *alpm_list_join(alpm_list_t *first, alpm_list_t *second) alpm_list_t *tmp; if (first == NULL) { - return second; + return(second); } if (second == NULL) { - return first; + return(first); } /* tmp is the last element of the first list */ tmp = first->prev; @@ -464,18 +465,22 @@ alpm_list_t SYMEXPORT *alpm_list_copy_data(const alpm_list_t *list, alpm_list_t SYMEXPORT *alpm_list_reverse(alpm_list_t *list) { const alpm_list_t *lp; - alpm_list_t *newlist = NULL; + alpm_list_t *newlist = NULL, *backup; - lp = alpm_list_last(list); - if(list) { - /* break our reverse circular list */ - list->prev = NULL; + if(list == NULL) { + return(NULL); } + + lp = alpm_list_last(list); + /* break our reverse circular list */ + backup = list->prev; + list->prev = NULL; while(lp) { newlist = alpm_list_add(newlist, lp->data); lp = lp->prev; } + list->prev = backup; /* restore tail pointer */ return(newlist); } @@ -490,14 +495,18 @@ alpm_list_t SYMEXPORT *alpm_list_reverse(alpm_list_t *list) */ inline alpm_list_t SYMEXPORT *alpm_list_first(const alpm_list_t *list) { - return((alpm_list_t*)list); + if(list) { + return((alpm_list_t*)list); + } else { + return(NULL); + } } /** * @brief Return nth element from list (starting from 0). * * @param list the list - * @param n the index of the item to find + * @param n the index of the item to find (n < alpm_list_count(list) IS needed) * * @return an alpm_list_t node for index `n` */ @@ -519,7 +528,11 @@ alpm_list_t SYMEXPORT *alpm_list_nth(const alpm_list_t *list, int n) */ inline alpm_list_t SYMEXPORT *alpm_list_next(const alpm_list_t *node) { - return(node->next); + if(node) { + return(node->next); + } else { + return(NULL); + } } /** -- 1.5.3.8
participants (3)
-
Nagy Gabor
-
solsTiCe d'Hiver
-
Xavier