0
Follow
2
View

Having trouble printing string from a file

ctrl61 注册会员
6 days ago

In the function add data members data of all nodes point to the same array buffer declared in main

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);
}

So the function print_list will output what is last stored in the buffer.

You need to make dynamically a copy of the passed string.

Also this while loop

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

does not make a great sense. First of all the variable ch shall be declared as having the type int. The condition in the loop

while(i < MAX-1 && !feof(file))

can result in storing the value EOF in the buffer. And the output demonstrates this.

(Edit: this update in your question

       if(ch == EOF)
              break;

you made after my answer.)

Also reading strings by one character is inefficient. You should use the function fscanf instead of fgetc.

Pay attention to that it is a bad idea to define the global variable head

ptr head = NULL;

and when some functions depend on the global variable.

Also in some parts of the program you are using the typedef name ptr while in other parts of the program you are using the type node *. This only confuses readers of the code.

dgangd 注册会员
6 days ago

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);
    }
}

About the Author

Question Info

Publish Time
6 days ago
Update Time
6 days ago