Pointer problem in C for char*

Question

Hi guys,

i use pointer for holding name and research lab property. But when i print the existing Vertex ,when i print the vertex, i cant see so -called attributes properly. For example though real value of name is "lancelot" , i see it as wrong such as "asdasdasdasd"

struct vertex {
                int value;
                char*name;
                char* researchLab;
                struct vertex *next;
                struct edge *list;
};
    void GRAPHinsertV(Graph G, int value,char*name,char*researchLab) {
    //create new  Vertex.
        Vertex newV = malloc(sizeof newV);
        // set  value of new variable  to which belongs the person.
        newV->value = value;
        newV->name=name;
        newV->researchLab=researchLab;
        newV->next = G->head;
        newV->list = NULL;
        G->head = newV;
        G->V++;
    }

    /***
    The method   creates new person.
    **/
    void createNewPerson(Graph G) {
        int id;
        char name[30];
        char researchLab[30];
        // get requeired variables.
        printf("Enter id of the person to be added.\n");
        scanf("%d",&id);
        printf("Enter name of the person to be added.\n");
        scanf("%s",name);
        printf("Enter researc lab of the person to  be added\n");
        scanf("%s",researchLab);
        // insert the people to the social network.
        GRAPHinsertV(G,id,name,researchLab);
    }
    void ListAllPeople(Graph G)
    {
        Vertex tmp;
        Edge list;
        for(tmp = G->head;tmp!=NULL;tmp=tmp->next)
        {
            fprintf(stdout,"V:%d\t%s\t%s\n",tmp->value,tmp->name,tmp->researchLab);

        }
        system("pause");
    }

Answer 1

The name variable you pass to GRAPHinsertV() is allocated on the stack for createNewPerson(), so the pointer points to a local variable. Once the activations records are popped off that value can (and will) be overwritten by subsequent code.

You need to allocate memory on the heap if you are only going to keep a char * in the struct.

Ex. Instead of

char name[30];

you could use

char *name = (char *)malloc(30*sizeof(char));

but keep in mind if you manually allocate it you have to take care of freeing it as well, otherwise it will have a memory leak.

Answer 2

When you assign the char *name pointer, like

newV->name=name;

You're not creating a new string, but making the newV.name member point to the same memory as the char[] array that was passed in. You'll need to malloc() or otherwise allocate a new char[] array in order to obtain separate storage for each structure.

Answer 3

GRAPHinsertV copies the pointer of the name and researchLab strings to the vector structure.

createNewPerson creates a temporary for the name and researchLab strings.

The problem here is, you're pointing to a temporary string which causes undefined behaviour when you access it after createNewPerson returns.

To solve this problem, you can duplicate the strings in GRAPHinsertV using malloc+strcpy, or by using the non-standard strdup.

Answer 4

There's a problem here:

Vertex newV = malloc(sizeof newV);

It should be

Vertex *newV = malloc(sizeof(Vertex));

Answer 5

When you do this:

   newV->name=name;
   newV->researchLab=researchLab;

You are copying the pointer to the strings name and researchLab. You are not copying the strings themselves. In other words, after this, newV->name and name point to exactly the same location in memory where the name is stored; you have not created a duplicate copy of the data.

Since you then proceed to overwrite the name array in the createNewPerson function, at the end of this function, all of your vertex structs will have their name attribute pointing to the same memory location, which is only storing the last name entered.

Worse, when createNewPerson returns, its local name array goes out of scope, and is re-used for other things. Since your vertex structs are still pointing here for their name attributes, this is how you get garbage.

You need to duplicate the string. A simple way to do it is:

newV->name = strdup(name);

You will need to #include <string.h> to get the strdup library function.

And then you also need to make sure that you call free on the name attribute whenever you are disposing of a vertex structure.

Answer 6

You are allocating memory in the function createNewPerson() that lasts exactly as long as createNewPerson() is executing, and is available for overwriting immediately after it returns. You need to copy the text fields in with something like strdup(newV->name, name), rather than point to the local variables in createNewPerson(). (If your implementation doesn't have strdup(), you can easily define it as:

char * strdup(const char *inp)
{
    char * s = malloc(strlen(inp) + 1);
    strcpy(s, inp);
    return s;
}

In addition, your I/O has potential problems. If you enter my name, "David Thornley", for the name, it'll take "David" as the name and "Thornley" as the lab, since "%s" searches for a whitespace-delimited string. If I enter "Forty-two" for the ID, nothing will be put in id, and "Forty-two" will be used for the name. If I enter a name or lab name over 29 characters, it will overwrite other memory.

I'd suggest using fgets() to get one line of input per answer, then use sscanf() to parse it.

Answer 7

When passing and assigning strings, always make a copy of them. There're no guarantees that the string you received is still in the memory afterwards, since the pointer could have been freed.

Of course, if you are only going to use name inside the function (that's, you're not going to assign it to a variable outside the scope of the function), you don't have to do the copy.

In order to do that, inside GRAPHinsertV, instead of

newV->name=name;

do

if (name != NULL)     // Preventing using null pointer
{
    newV->name = malloc(strlen(name)+1);
    strcpy(newV->name, name);
}