Search code examples
cdynamic-memory-allocationc-stringssingly-linked-list

Having trouble printing string from a file


I have a task to read words from a file and print each word separately. So far I got this:

#define MAX 100000
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct node *ptr;
typedef struct node {
    char *data;
    ptr next;
} node;

ptr head = NULL;

void add(char *data) {
        node *new_node = malloc(sizeof(node));
        new_node->data = data;
        new_node->next = head;
        head = new_node;
    printf("Adding new word:%s\n",data);
}

void print_list() {
    node *current = head;
        while (current != NULL) {
            printf("%s\n", current->data);
            current = current->next;
        }
}

void free_list(ptr *hnode)
{
    ptr p;
    while(*hnode){
        p = *hnode;
        *hnode = (*hnode)->next;
        free(p);
    }
}

int main(int argc, char *argv[]) {
    FILE *file;
    char buffer[MAX];
    char ch;
    int i = 0;

    if (argc != 2) {
        printf("Usage: %s <file>\n", argv[0]);
            exit(EXIT_FAILURE);
        }

        file = fopen(argv[1], "r");
        if (file == NULL) {
            printf("Error opening file\n");
            exit(EXIT_FAILURE);
        }

    while(i < MAX-1 && !feof(file))
    {
        if((ch = fgetc(file)) == ' '|| ch == '\n')
        {
            buffer[i] = '\0';
            add(buffer);
            i = 0;
        }
        else{
            if(ch == EOF)
                  break;
            buffer[i++] = ch;
        }
    }

        print_list();
        free_list(&head);
        exit(EXIT_SUCCESS);
}

When I try to run the code there are no compilations or runtime errors but I get only the last word and a weird symbol, for example:

Adding new word:Hello
Adding new word:World!
Adding new word:Does
Adding new word:my
Adding new word:code
Adding new word:work?
work?
work?
work?
work?
work?
work?

The part when it says adding new word is correct and reads the file properly, my problem is the printing part. Edit: Now I have a proper EOF check, still doesn't work as intended.


Solution

  • When you call add(buffer), you're passing the exact same pointer every time (technically, arrays aren't pointers but it's close enough for this discussion). That pointer gets assigned to the data field of your nodes. This means that every node contains a reference to the exact same string. That's why every node is printing out the same value. In addition, as your code becomes more complex, you run the risk of these references becoming invalid if buffer's lifetime ever expires.

    What you want to do is use malloc to give each node its own copy of the string.

    void add(char *data) {
        size_t len = strlen(data);
        node *new_node = malloc(sizeof(node)); // Note: You should check this return value.
        new_node->data = malloc(len+1); // Note: Ditto.
        memcpy(new_node->data, data, len+1);
        new_node->next = head;
        head = new_node;
        printf("Adding new word:%s\n",data);
    }
    
    void free_list(ptr *hnode)
    {
        ptr p;
        while(*hnode){
            p = *hnode;
            *hnode = (*hnode)->next;
            free(p->data);
            free(p);
        }
    }