freeing up memory from a struct gives invalid pointer

Question

I am writing some experimental code, to get better at C, I wrote below code

/*This is a test code. Some of the checks are not performed.*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

struct person {
    char *name;
    char *occupation;
    int age;
};

typedef struct person prn;

int main(void)
{
    prn *ptr=NULL;
    ptr = malloc(sizeof(ptr));//a node size of prn is created. No need to do malloc(sizeof(prn)) as variable also have same type.
    ptr->name = malloc(sizeof(ptr->name) * ( strlen("Mrigendra") + 1));//
    ptr->occupation = malloc( sizeof( ptr->occupation) * (strlen("SoftwareE")+1) );

    ptr->name = "Mrigendra";//should use memcpy.
    ptr->occupation = "SoftwareE";
    ptr->age = 20;

    printf("%s\n", ptr->name);
    printf("%s\n", ptr->occupation);
    printf("%d\n", ptr->age);

    free(ptr->occupation);
    free(ptr->name);
    free(ptr);
    return 0;

}

On executing above program I get this error

Mrigendra
SoftwareE
20
*** Error in `./a.out': free(): invalid pointer: 0x000000000040076e ***
Aborted (core dumped)

I am in this thinking that ptr is pointing to a location on heap. Also its members are also pointers and pointing to the heap, so I freed members and then free the ptr.

1.One thing come to my mind is that 'age' is also on heap and I haven't freed it. Is this the reason?

2.What is the correct way to debug it? (I have a fear its a dumb question or something obvious I missed)

3."core is dumped" where?

Answer 1

First, ptr = malloc(sizeof(ptr)) should be ptr = malloc(sizeof(prn)) in order to allocate enough memory to hold an object of type struct person. sizeof(ptr), in contrast, would always be 8 (the size of a pointer value on a 64 bit system).

Second, ptr->name = malloc(sizeof(ptr->name) * ( strlen("Mrigendra") + 1)); should be ptr->name = malloc(strlen("Mrigendra") + 1) in order to allocate exactly the memory to hold the string Mrigendra + the string termination character.

Third, instead of ptr->name = "Mrigendra", which lets ptr->name point to a string literal, use strcpy(ptr->name,"Mrigendra"). Otherwise, you will later free a pointer to a string literal, which is undefined behaviour.

Note: instead of malloc + strcpy, most systems offer a function ptr->name=strdup("Mrigendra"), which does an appropriate malloc and the strcpy in one function.

Answer 2

sizeof(ptr) needs to be changed to sizeof(*ptr). sizeof(ptr) only returns the size of a pointer on your machine, so when you set the members of the struct you're getting a heap overflow and probably writing over malloc's bookkeeping information.

Answer 3

C-string assignment does not work with the = operator.

ptr->name = "Mrigendra";

In this line, the char[] literal decays to a char* and then that pointer is assigned to ptr->name. The same is true for the rest of your assignments. The memory you malloced is leaked here.

When you reach the free statements in your code, you attempt to free the address of the string literals, not the memory you allocated earlier.

To get around this, use a function that properly copies the content like

strcpy(ptr->name, "Jane");

---

I missed a second mistake. Read this answer for an explanation. As C_Elegans points out

sizeof(ptr) needs to be changed to sizeof(*ptr)

Answer 4

ptr = malloc(sizeof(ptr));

The above line of code will not allocate enough memory for the struct, you should use instead:

ptr = malloc(sizeof(*ptr));

or similar.

You allocate memory for the struct pointers, but then, you overwrite their values by assigning string literals to them, losing the addresses of the allocated memory.

So, now you have memory leak, and when you try to call free() upon them, you have undefined behavior. You should use functions like strcpy() or memcpy() to copy strings, not the assignment operator '='.