Nsryjdtyk

I'm having an issue with assigning a null value to a pointer to a struct member (that is also a pointer to a struct). The following code correctly sets the headNode->right variable equal to null, but not the headNode->left variable.

typedef struct node {

    char *key;

    int frequency;

    struct node *left;

    struct node *right;

    struct node *parent;

} node;





void addKey(char key) {

    extern node *headNode;

    node *newNode;



    if (headNode != NULL) {

        printf("Head node is initializedn");

        if (headNode->left != NULL) printf("    Left is not nulln");

        if (headNode->right != NULL) printf("    Right is not nulln");

    }





    newNode = malloc(sizeof(node*));

    newNode->key = malloc(sizeof(char) * (strlen(key) + 1));

    newNode->left = malloc(sizeof(node*));

    newNode->right = malloc(sizeof(node*));

    newNode->parent = malloc(sizeof(node*));





    newNode->left = newNode->right = NULL;

    newNode->frequency = 1;

    newNode->right = NULL;

    strcpy(newNode->key, key);



    // If this is the first node, assign it as the root

    if (headNode == NULL) {

        newNode->parent = NULL;

        headNode = newNode;

        return;

    }

}

However, if I add the following two lines right before the return statement, it works correctly.

if (headNode->left == NULL) printf("L is nulln");

else printf("L is NOT nulln");

I don't understand how an if statement can make a difference. There is nowhere else that assigns or changes the value of this variable anywhere else in my code.

Take a step back. There are a number of serious mistakes here.

You declare a pointer to a node:

node *newNode;

So far, so good. However, when you try to allocate memory for this new node, you only allocate enough to store a pointer, instead of the node itself:

newNode = malloc(sizeof(node*));

You already have storage for the pointer: newNode.

Change this line to:

newNode = malloc(sizeof(node));

A little later you make three more unnecessary calls to malloc:

newNode->left = malloc(sizeof(node*));

newNode->right = malloc(sizeof(node*));

newNode->parent = malloc(sizeof(node*));

If you had requested enough memory (i.e. sizeof(node)) in the previous call, there would be no need to allocate any more memory for these internal fields. You would already have memory for them; it would already exists at the address pointed to by newNode.

Worse still, you then go on to leak memory on the next line:

newNode->left = newNode->right = NULL;

Here you are throwing away the addresses of the memory you allocated unnecessarily.

I think you need to read up on memory management further before we can troubleshoot this problem for you.

All the malloc(sizeof(node*)) should be malloc(sizeof(node)). You're only alloocating enough space for a pointer, not the whole structure. This is causing undefined behavior with all the following code that indirects through these pointers.