Struct variables getting overwritten when writing to local variables

Hi all,

I'm relatively new to C and can't for the life of me figure out what I'm doing wrong! I have a simple struct setup to create a binary tree with a char * called 'name'. I also have a local variable called 'name' of the same type. When I write to the local variable 'name' the version in the struct also gets written to. I have no idea why this is but I'm guessing it's something to do with how the memory is being written to. The code below is a very simple example of what is going wrong:

Code:

---

#include <stdio.h>

#define MAX_INPUT_SIZE 100

int main() {
       
        struct node {
                char *name;
                struct node *left;
                struct node *right;
        };
       
        struct node *root;
        root = NULL;
        root = malloc(sizeof(struct node));
        root->name = NULL;
       
        char *name;
        /* Stores user input */
        char in[MAX_INPUT_SIZE];
        /* Used for checking presence of '\n' in expr */
        char *p;
        /* Used to check if user has finished inputting numbers */
        int exp_in = 1;
       
        /* Loop whilst the user wants to add more names */
        while (exp_in) {
                if (fgets(in, sizeof(in), stdin) != NULL) {
                       
                        /* Remove '\n' if fgets adds it */
                        if ((p = strchr(in, '\n')) != NULL)
                                *p = '\0';
               
                        /* Check to see if user wants to exit */
                        if (strcmp(in, ".") == 0) {
                                exp_in = 0;
                        /* Otherwise handle the input and add to tree */
                        } else {
                                /* Loop counter, variable to store string length and variable to store i at start of phone number */
                                int i, len = strlen(in), j;
                               
                                /* Get name from input */
                                for (i=0; (i < len) && (in[i] != ' '); i++)
                                        name[i] = in[i];
                                       
                                name[i] = '\0';       
                               
                                printf("%s\n", (root->name != NULL)?root->name:"");
                               
                                root->name = name;
                        }
                }
        }
}

---

Can anyone offer any ideas on what could be going wrong?

Cheers,
James

'name', the local copy, doesn't actually point anywhere valid--you never allocate memory for it.
'root-name' is being told to point at local 'name'. You're just sharing the address. It's like saying:

"Two people live here, and when one of them made the house messy, it made the house messy for the other guy too!"

Of course it did, they're sharing the same house! If you want them to have different houses, one of them needs a different address!


Quzah.

Always make your compiler output warning. In GCC add -Wall. Because you need:

Code:

---

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

---

Quzah: Thanks, I see what I'm doing wrong now - I knew it would be something simple!

C_ntua: Thanks for pointing that out - my real code does include all the correct headers it's just I forgot to include some of them in the quick example I did!

Hi again,

I've just tried what I think you suggested - I made my local name an array of characters and then malloc'ed enought space to hold name before using it within my struct however I still get the same problem... I'm guessing there is soomething I'm missing, my code is below:

Code:

---

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

#define MAX_INPUT_SIZE 100

int main() {
       
        struct node {
                char *name;
                struct node *left;
                struct node *right;
        };
       
        struct node *root;
        root = NULL;
        root = malloc(sizeof(struct node));
        root->name = NULL;
       
        char name[MAX_INPUT_SIZE];
        /* Stores user input */
        char in[MAX_INPUT_SIZE];
        /* Used for checking presence of '\n' in expr */
        char *p;
        /* Used to check if user has finished inputting numbers */
        int exp_in = 1;
       
        /* Loop whilst the user wants to add more names */
        while (exp_in) {
                if (fgets(in, sizeof(in), stdin) != NULL) {
                       
                        /* Remove '\n' if fgets adds it */
                        if ((p = strchr(in, '\n')) != NULL)
                                *p = '\0';
               
                        /* Check to see if user wants to exit */
                        if (strcmp(in, ".") == 0) {
                                exp_in = 0;
                        /* Otherwise handle the input and add to tree */
                        } else {
                                /* Loop counter, variable to store string length and variable to store i at start of phone number */
                                int i, len = strlen(in), j;
                               
                                /* Get name from input */
                                for (i=0; (i < len) && (in[i] != ' '); i++)
                                        name[i] = in[i];
                                       
                                name[i] = '\0';       
                               
                                printf("%s\n", (root->name != NULL)?root->name:"");
                               
                                root->name = malloc((strlen(name))*sizeof(char));
                                root->name = name;
                        }
                }
        }
}

---

Code:

---

root->name = malloc((strlen(name))*sizeof(char));
root->name = name;

---

All you're doing again is making root->name point at name. You can't assign strings or arrays that way:

Code:

---

root->name = malloc( somesizehere );
strcpy( root->name, fromhere );

---

Or you could use strncpy to copy a specific amount of characters. But you need to make sure you null terminate on your own that way.


Quzah.

Right, I understand now - I keep getting caught out with not using strcpy! I'm used to programming in PHP which allows you to assign strings using the '=' operator and I'm finding it hard remembering not to do it in C!

Thanks again,
James