Data Loss when trying to copy char* in C

Question

I have been working on a project in C and I am having trouble when trying to copy char* using strcpy/memcpy/strncpy, none of these seem to work. The problem that is arising is that the words that are around 8 or more characters long are not being copied completely.

typedef struct wordFrequency {
    char * word;
    int frequency;

struct wordFrequency *left, *right;
} *node;


node setnode(char * word) {

    node newNode = (node)malloc(sizeof(node));
    newNode->word = (char*)malloc(sizeof(word));

    strcpy(newNode->word, word); //This is where I'm having trouble

    newNode->frequency = 1;
    newNode->right = NULL;

    return newNode;
}

The code above is what I believe is the main cause for error, but I don't know where to fix it. I have tried messing with the sizes, but that didn't work.

If possible can someone explain to me a way to copy all characters or if I did not allocate enough space?

Answer 1

This program is an mcve that shows how to properly allocate and initialize each node in your linked list:

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

#define ARRAY_SIZE(array) \
    (sizeof(array) / sizeof(array[0]))

typedef struct wordFrequency {
    char *word;
    int frequency;
    struct wordFrequency *left, *right;
} node;

node *setnode(char *word) {
    node *newNode = malloc(sizeof(node));
    newNode->word = malloc(strlen(word) + 1);
    strcpy(newNode->word, word);
    newNode->frequency = 1;
    newNode->right = NULL;
    return newNode;
}

int main() {
    char *wordList[] = {"one", "two", "three"};
    node nodeHead;
    node *nodePrev = &nodeHead;
    node *nodeNext;
    for (int index = 0; index < ARRAY_SIZE(wordList); index++) {
        nodeNext = setnode(wordList[index]);
        nodePrev->right = nodeNext;
        nodeNext->left = nodePrev;
        nodePrev = nodeNext;
    }
    for (node *nodePtr = nodeHead.right; nodePtr != NULL; nodePtr = nodePtr->right) {
        printf("word = %s, frequency = %d\n", nodePtr->word, nodePtr->frequency);
    }
    return 0;
}

Output

word = one, frequency = 1
word = two, frequency = 1
word = three, frequency = 1

Note

This program has no error checking and does not free the allocated memory. This code should not be used in a production environment.

Replies to Questions in Comments

I replaced *node with node in the typedef because that allows me to declare instances of node. The other syntax only allows pointers to node.

I use an instance of node instead of node * for nodeHead because any attempt to change its address will be an error.

I use nodePrev to traverse the list and also to provide a target for left in the returned nodes. I initialize nodePrev to &nodeHead because it is the start of the list. I set nodePrev to nodeNext because that's how I chose to traverse the list during initialization. I could have used

nodePrev = nodePrev->right;

and achieved the same effect.

I only implemented list handling so that I could create a self-contained example that would run without changes. You can safely ignore it.

If you want to see good linked list code, I recommend the linux kernel implementation.