How to print out the mini strings in a string divided by '.' using linked lists in c

Question

WE are given a string which is "divided" by '.' and we have to use linked lists to print out the parts of the string

My problem are 2

1. The first is that in the parts im printing there is a wierd type of character which i normally associate with error. Basically something that should not be printed

2. The last node doesnt print and i cant figure out how to add it

This is my code

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

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


node_t *split_string(char *str)
{
    int i=0,j=0;
    int k=0;
    int count=0;
    node_t *head=NULL,
            *tmp=NULL,
            *p=NULL;
    char buffer[20];
    for(i=0;i<strlen(str);i++)
    {
        if(str[i]!='.')
            count++;

        else
        {

            for(j=i-count;j<i;j++)
            {
                buffer[k]=str[j];
                k++;
            }
            k=0;
            count=0;
            tmp=(node_t*)malloc(sizeof(node_t));
            if(tmp == NULL)
            {
                exit(1);
            }
            tmp->next=NULL;
            tmp->word=strdup(buffer);
            if(tmp->word == NULL)
            {
                exit(2);
            }
            if(head == NULL)
            {
                head=tmp;
            }
            else
            {
                p=head;
                while(p->next != NULL) // I know that the problem is here but if i switch   
                                       // it to while(p != NULL) it wont print anything
                {
                    p=p->next;
                }
                p->next=tmp;
            }
        }
    }
    return head;

}
void printList(node_t *head)
{
    node_t *tmp=head;
    int i=1;
    while(tmp != NULL)
    {
        printf("Node %d -> %s\n",i,tmp->word);
        tmp=tmp->next;
        i++;
    }
}
int main()
{
    char str[]={"aaa.sss.ddd.aaa.ddd.ddd"};
    node_t *head=NULL;
    head=split_string(str);
    printList(head);
    return 0;
}

Answer 1

1. I suggest you refactor your split_string() into smaller functions: create_node() and append_list(). Use libc functions like strchr() instead of writing your own.

2. For good measure I also implemented the optimization that @Somprogrammerdude mention of keeping track of the tail.

3. The _t suffix is a reserved identifier so don't use that.

4. Don't cast the (void *) from malloc().

5. Prefer for to while loops when iterating.

6. Use consistent naming either camel printList() or snake case (split_string()). The only evidence that I have seen is that snake case it slightly faster to read so that is what I use.

7. Use const for pointer arguments when the value doesn't change.

8. Minimize scope of variables, and consider eliminating variables. State is what makes code hard to reason about.

9. Write a free_list() functions that free() each word and node.

#define _POSIX_C_SOURCE 200809L
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct node {
    char *word;
    struct node *next;
} node;

node *create_node(const char *s) {
    node *node = malloc(sizeof *node);
    if(!node) {
        printf("malloc() failed\n");
        return NULL;
    }
    node->word = (char *) s;
    if(!s) {
        printf("s is NULL\n");
    }
    node->next = NULL;
    return node;
}

node *append_list(node *n, const char *s) {
    node **p = &n;
    for(; *p; p = &(*p)->next);
    return *p = create_node(s);
}

node *split_string(const char *str) {
    node *head = NULL;
    node *tail = NULL;
    for(const char *sep = str; sep; str = sep + 1) {
        sep = strchr(str, '.');
        tail = append_list(tail, sep ? strndup(str, sep - str) : strdup(str));
        if(!head)
            head = tail;
    }
    return head;
}

void print_list(node *head) {
    for(size_t i = 0; head; i++, head = head->next)
        printf("Node %zu -> %s\n", i, head->word);
}

void free_list(node *head) {
    while(head) {            
        free(head->word);      
        node *tmp = head;
        head = head->next;
        free(tmp);
    }
}

int main() {
    node *head = split_string("aaa.sss.ddd.aaa.ddd.ddd");
    print_list(head);
    free_list(head);
}

and example output:

Node 0 -> aaa
Node 1 -> sss
Node 2 -> ddd
Node 3 -> aaa
Node 4 -> ddd
Node 5 -> ddd

Answer 2

This is because you are only looking for a '.' as the terminator of a node. The last node ends with a null terminator. Try the following:

for(i=0; i < strlen(str)+1; i++) // allow the loop to include the null termintor
{
    if(str[i] != '.' && str[i] != 0) // check for end of string as well
        count++;
    else {
        // do you stuff
    }
}

BTW, the code can be refactored.