CODEWITHSUNDEEP
clinked-list
DTSCode
DTSCode

Reputation: 1100

Linked list not printing all values

I am writing a linked list as part of a larger project, and have run into an issue. This testcase takes a string of comma delimited ranges (where a range in this case is either an integer or two integers separated by a dash), and add each range into a singly linked list. Currently, it's only printing the first two ranges, and I can't see why. Here is the code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main() {
    char port_list[] = "22-25,80,443-445,4200-4205";

    struct range_list {
        struct range_list *next;
        char *range;
    };

    struct range_list *head = (struct range_list*) malloc(sizeof(struct range_list));
    head->next = 0;
    head->range = strtok(port_list, ",");

    struct range_list *iter = (struct range_list*) malloc(sizeof(struct range_list));
    head->next = iter;
    iter->next = 0;

    while((iter->range = strtok(NULL, ",")) != NULL) {
        iter = (struct range_list*) malloc(sizeof(struct range_list));
        iter->next = iter;
        iter->next = 0;
    }

    for(iter=head; iter != 0; iter=iter->next) {
        printf("%s\n", iter->range);
    }
}

Currently, the output is:

22-25
80

And ideally I would like:

22-25
80
443-445
4200-4205

Thanks for any help in advance! I'm sure it's a simple issue. I just get scared writing linked lists, so am very out of practice.

Upvotes: 0

Views: 50

Answers (3)

granmirupa
granmirupa

Reputation: 2790

Here you are overwriting iter:

 while((iter->range = strtok(NULL, ",")) != NULL) {
        iter = (struct range_list*) malloc(sizeof(struct range_list)); // you are overwriting iter;
        iter->next = iter;
        iter->next = 0;
    }

Try this:

struct range_list* tmp;
struct range_list* last;



while((iter->range = strtok(NULL, ",")) != NULL) {
    last = iter;
    tmp = (struct range_list*) malloc(sizeof(struct range_list)); 
    iter->next = tmp;
    iter = iter->next;
    iter->next = 0;
}
free(last->next);
last->next = 0;


for(iter=head; iter != 0; iter=iter->next) {
    printf("%s\n", iter->range);
}

Upvotes: 0

totoro
totoro

Reputation: 2456

This seems to work. I think it's basically the same everyone is saying in different words.

Ideone.com

char port_list[] = "22-25,80,443-445,4200-4205";
char *range;
// Create the head and start the tokener.
struct range_list *iter, *head = calloc(1, sizeof(struct range_list));
head->range = strtok(port_list, ",");

// As long as there is a token append a new item.    
while((range = strtok(NULL, ",")) != NULL) {
    iter = (struct range_list*) malloc(sizeof(struct range_list));
    iter->next = head;
    iter->range = range;
    head = iter;
}

for(iter=head; iter != 0; iter=iter->next) {
    printf("%s\n", iter->range);
}

OUTPUT

4200-4205
443-445
80
22-25

Upvotes: 0

Christophe
Christophe

Reputation: 73627

Your code has an issue if there's only one range in the string.

I'll therefore propose you a cleaner alternative: 

...
// as before, assuming ther's at least one range
struct range_list *head = (struct range_list*) malloc(sizeof(struct range_list));
head->next = 0;
head->range = strtok(port_list, ",");

struct range_list *iter=head, *tmp; 
char *s; 

while((s = strtok(NULL, ",")) != NULL) {
    // there is a new item.  So first create and initialise it 
    tmp = (struct range_list*) malloc(sizeof(struct range_list)); 
    tmp->next=0; 
    tmp->range=s; 
    // then link it to the previous and interate
    iter->next = tmp;
    iter = iter->next;
}
...

This one assumes that there is at least one range in the string to be put into head.

Online demo

Note that the range pointers point to the original buffer. Here it's not a problem, because the original buffer remains unchanged. You may consider strdup() if you're on a posix system to make a safe copy of your range string if there's a risk that your linked list lives longer than the buffer or its content.

Upvotes: 2

Related Questions