why is that the for loop terminates but the memcpy within copies the same substring?

Question

My assignment asks us to shorten the words of an user input sentence. Then put the shortened words into a linked list. When the gears of code get stuck: the last word in the sentence gets saved into every node. What repair should be implemented so the code puts a diff word into a node each time.

note: I looked at questions involving memcpy & implemented using 0 and\0 to fill the arrays beforehand. I've also changed the arguments of memcpy with & and without&. I also used memmove and strncpy. This Variation of 'memcpy in for loop to copy substring of array into node' has not been asked.

Here is my code. Would appreciate recommendations for learning resources links to c or java in the comments of your answer. As well as suggestions for improving code. Of course if you have a more concise/accurate version of my question I will gladly update it. Thank you!

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

typedef struct node
{
    char *four_letters; 
    struct node *next_node;
}node;

struct node* head= NULL; 

char sentence[120][40]={0}, four_letters[40]={0}; // Tried \0,"0".

int word, num_words=-1; 

void scan(); void print(); void sentence_into_list();

void add(struct node **head, char *four_letters); 

void display(struct node *head);



int main()
{
    scan();
    printf("\n");
    print();
    printf("\n");

    sentence_into_list();
    display(head);
    return 0;
}



void scan()
{
   for(word=0;;word++)
    {
        scanf("%s",sentence[word]);
        num_words++;
        if(getchar()=='\n')
            break;
    }

}

void print()
{
    for(word=0;word<=num_words;word++)
    {
        printf("%s ", sentence[word]);
    }
}

void sentence_into_list()
{   
    for (word=0;word<=num_words;word++)
    {
        memcpy(four_letters, sentence[word], 4);     //tried & //tried strncpy, memmove.
        add(&head, four_letters);
    }
}

void add(struct node **head, char *four_letters)

{
    struct node *new_node = malloc(sizeof(struct node));

    new_node->four_letters = four_letters;
    new_node->next_node = *head;
    *head=new_node;

}

void display(struct node* head)
{
    struct node *current;
    current = head;
    if(current!=NULL)
    {
        printf("List:");
        do
        {
            printf("%s ",current->four_letters);
            current = current->next_node;
        }

        while(current!=NULL);
            printf("\n");
    }
        else
        {
            printf("empty\n");
        }
}

Answer 1

the last word in the sentence gets saved into every node. ....

This is happening because four_letters pointer of all the nodes of the list are pointing to the same memory location four_letters [char buffer declared globally - four_letters[40]={0};]. Hence, whatever is the last value of four_letters will be reflected in all nodes.

One way of solving this problem is -
allocate a memory, large enough to hold the data, to the node four_letters pointer and copy the content of four_letters to it. Alternatively, you can use strdup which does this for you.

So, in the add() function, replace this

new_node->four_letters = four_letters;

with this

new_node->four_letters = strdup(four_letters);

strdup creates a duplicate of the string passed to it. It returns a pointer to newly allocated memory so, you should free it once you are done with it, like this:

free (node_ptr->four_letters); //node_ptr is pointer to current node

While freeing the dynamically allocated memory of list nodes, make sure to first free the dynamically allocated members of the node structure and then free the node itself.

The other way of solving this problem is -
Instead of having a pointer to the char in node structure, have the char array as the member of node structure, like this:

typedef struct node
{
    char four_letters[5]; // 5 because it is suppose to keep the four letters only and +1 is for null-terminating character
    struct node *next_node;
}node;

and copy the content of four_letters passed to add() to the structure node member four_letters. With this, you don't need to take care of freeing the memory.

There is one more problem with your code -
In the function sentence_into_list(), you are doing:

    memcpy(four_letters, sentence[word], 4);
    add(&head, four_letters);

Here you're copying the first 4 characters of sentence[word] to four_letters but you should also add the null-terminating character at the 5th location of the buffer four_letters before passing it to add(), something like this:

four_letters[4] = '\0';

C language does not have the native string type. In C, strings are actually a one-dimensional array of characters terminated by a null character \0.

You are not seeing any problem in your code because you are initializing the buffer with zeros:

four_letters[40]={0};

and every time you are only copying only the 4 characters to it. So, at the 5th location you always have \0. But assume a case where you are using the buffer for different size of strings. In such case, you must have to place the terminating null-character at the end of the string.

---

Additional:

I can see that you are nowhere freeing the memory allocated dynamically for the nodes of the list. Follow the good programming practice, make a habit of freeing the dynamically allocated memory once you are done with it.