simple list iteration

hi,

i have something like this:

Code:

typedef struct node {
        node *next;
        char  *data;
} node_t;

can anyone please post a piece of code that will iterate through a list construced of nodes, given a head element 'head'?

my code:

Code:

int print_history(hashnode_t *node){
        hashnode_t *tmp = node;
        while (tmp != NULL) {
                printf("%s ", tmp->data);
                tmp = tmp->next;
        }
        return 0;
}

this works good, however always goes one element too far and thus prints a 'null' at the end of the list. i do not understand why.

i malloc the next elements like this:

Code:

current->next = (hashnode_t *) malloc(sizeof(hashnode_t)); //create next

thanks for any explanations and help.

felix

It's almost certainly a problem with the code which generates the list in the first place.
Typically, this is caused by having a dummy node at one end or the other, or perhaps problems with file reading code using feof() inappropriately.

There is nothing wrong with print_history() if your list is constructed properly.

Oh, and there's no need to cast the result of malloc in C.
See the FAQ for why.

Originally Posted by zedoo

Code:

current->next = (hashnode_t *) malloc(sizeof(hashnode_t)); //create next

This seems to be ok. The question is just when you allocate space for the next node. If you do this for the new node when you create it then it's too early . Need to see the code for inserting data into your list.
Kurt

ok this is it. basically i read something from stdin, make a hash of it and store that hash + the name inside a node. if i insert "h" then i dont want to hash the "h" but instead just print the history of hashes.

Code:

int main(int argc, char** argv)
{
        char in[ASAP_BUFSIZE];
        printf("**ASAP SHELL**\n>");
        hashnode_t head;
        hashnode_t *current = &head;
        while (TRUE){
                in[0] = '\0'; //reset buffer
                if (fgets(in, ASAP_BUFSIZE, stdin) == NULL){
                        continue;
                }
                int length  = strlen(in);
                in[length-1] = '\0';  //delete newline from input
                if (length == ASAP_BUFSIZE - 1){
                        printf("damn: buffer too small\n>");
                        continue;
                }
                if (strcmp(in, CMD_HISTORY) == 0){
                        print_history(&head);
                        continue;
                }
                char *in_copy = (char *) malloc((length + 1)*sizeof(char));
                strcpy(in_copy, in);
                current->filename = in_copy;
                sha1_from_file(in, current->md);
                current->next = (hashnode_t *) malloc(sizeof(hashnode_t)); //create next element
                printf("malloc\n");
                current = current->next;
                printf(">");
        }
}

You are alwais creating a new node in advance. So your code to iterate the list should be

Code:

int print_history(hashnode_t *node){
        hashnode_t *tmp = node;
        while (tmp != current ) {
                printf("%s ", tmp->data);
                tmp = tmp->next;
        }
        return 0;
}

But that would mean that you have to make current global. Another solution would be to set data to 0 when you create the node and check for data == 0 when iterating.
Kurt

Code:

if (fgets(in, ASAP_BUFSIZE, stdin) == NULL) {
    continue;
}

If fgets() returns NULL, don't continue. I think you should break.

Originally Posted by ZuK

You are alwais creating a new node in advance...

Thanks ZuK, this is really the cause.

Sure is a lot of C++ in that C code.
The malloc casts are a giveaway.

I bet you get warnings containing "void*" if you remove those casts - right?

Indeed I get no warnings when i remove those casts, and i compile with -Wall

Originally Posted by Salem

Sure is a lot of C++ in that C code.

Hmm, yes i am not a C programmer. What are you exactly meaning? Variable declarations within the code?

zedoo, try using recursive function

Code:

struct NODE  {
  char * data;
  struct NODE * next;
}

typedef struct NODE NODE;
typedef NODE * NODE;

insert(NODE * node, char * data)
{
  if(*node == NULL)  {
    *node = (struct NODE*)malloc(sizeof(struct NODE));
    if(*node != NULL)  {
       (*node)->data = data;
       (*node)->next = NULL;
    }
     else
       assert(0);
  }
  else  {
    insert(&((*node)->next),data);
  }
}


in you main menu
main()
{
    NODE node = NULL;
    insert(&node, some data here);



}

> Variable declarations within the code?
Yes.
The ability to do that is a new feature of C99 (the new standard for C).

Since you mention -Wall, I assume you're using the GNU compiler. As far as I know, gcc will accept this style of programming without comment unless you also specify -ansi as well. On the other hand, if you're using the very latest version perhaps it defaults to C99 mode to begin with.

I normally use
gcc -W -Wall -ansi -pedantic -O2 prog.c
to flush out quite a lot of problems.

> zedoo, try using recursive function
Ah, but what about stack space and very long lists?