5

I am learning C for malloc and free heap memory related to self-defined structure.

The problem I met

For below code I constantly got segment fault. Using valgrind to analysios showed : ==12999== Address 0x108b4b is in a r-x mapped file <folder_path> segment ==12999==

full output :

Name : Amy 
Date of Birth: 1989 9 21
Name : echo 
Date of Birth: 1989 9 21
Name : echo 
Date of Birth: 1989 9 21
Name : echo Address 0x5580be70e270
Date of Birth: 1989 9 21
Their address : 0x5580be70e260 0x5580be70e264 0x5580be70e268
Name : echo Address 0x5580be70e270
Date of Birth: 1989 9 21
Their address : 0x5580be70e260 0x5580be70e264 0x5580be70e268
free p->name 
Segmentation fault (core dumped)

The analysis I did

I use valgrind to try to find where results this: valgrind --tool=memcheck --leak-check=full ./person2 The output is shown as :

Their address : 0x522d040 0x522d044 0x522d048
Name : echo Address 0x522d050
Date of Birth: 1989 9 21
Their address : 0x522d040 0x522d044 0x522d048
free p->name 
==12999== Invalid free() / delete / delete[] / realloc()
==12999==    at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12999==    by 0x108894: Person_destruct (person2.c:33)
==12999==    by 0x108A2F: main (person2.c:74)
==12999==  Address 0x108b4b is in a r-x mapped file /folder/path segment
==12999== 
freed p->name 
free p 
freed p 
==12999== 
==12999== HEAP SUMMARY:
==12999==     in use at exit: 4 bytes in 1 blocks
==12999==   total heap usage: 3 allocs, 3 frees, 1,052 bytes allocated
==12999== 
==12999== 4 bytes in 1 blocks are definitely lost in loss record 1 of 1
==12999==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12999==    by 0x108817: Person_construct (person2.c:16)
==12999==    by 0x1089CC: main (person2.c:64)
==12999== 
==12999== LEAK SUMMARY:
==12999==    definitely lost: 4 bytes in 1 blocks
==12999==    indirectly lost: 0 bytes in 0 blocks
==12999==      possibly lost: 0 bytes in 0 blocks
==12999==    still reachable: 0 bytes in 0 blocks
==12999==         suppressed: 0 bytes in 0 blocks
==12999== 
==12999== For counts of detected and suppressed errors, rerun with: -v
==12999== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

What I understand is that *p3 points to the exact same memory piece as p1 (which proved by Person_address_print(), thus I only explicitly destruct p1 to avoid twice destruct the same heap memory.) I also tried to let p3->name = "ada"; to ensure the allocated string is still within the previous length (3) or explicitly set p3 == NULL before calling Person_destruct(p1); the same segment fault error occurs :(

The code

Here is the code. The code basically does: a. self-defined a structure Person b. (in main)construct objects for Person, destruct them and print related attributes.

//  person.h
#ifndef PERSON_H
#define PERSON_H
typedef struct
{
    int year;
    int month;
    int date;
    char * name; // name is a ponter b/c name length is unknown
} Person;
Person * Person_construct(int y, int m, int d, char * n);
void Person_destruct(Person *p);
void Person_print(Person * p);
#endif
#include "person.h"
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
Person *Person_construct(int y, int m, int d, char *n)
{
    Person *p = NULL;
    p = malloc(sizeof (Person));
    if (p == NULL) // malloc failed
    {
        return NULL;
    }
    p->year= y;
    p->month = m;
    p->date = d;
    p->name = malloc(sizeof(char) * (strlen(n) + 1));
    // + 1 for the neding char '\0'
    if ((p->name) == NULL)// malloc failed, just return
    {
        free(p); 
        return NULL;
    }
    strcpy(p->name, n);
    return p;
}

void Person_destruct(Person *p)
{
    // p->name must be freed befoer p is freed
    if (p->name != NULL)
    {
        printf("free p->name \n");
        free(p->name);
        printf("freed p->name \n");
    }

    if (p != NULL)
    {
        printf("free p \n");
        free(p);
        printf("freed p \n");
    }
}

void Person_print(Person *p)
{
    printf("Name : %s \n", p->name);
    printf("Date of Birth: %d %d %d\n", 
        p->year, p->month, p->date);
}


void Person_address_print(Person *p)
{
    printf("Name : %s Address %p\n", p->name, &(p->name));
    printf("Date of Birth: %d %d %d\n", 
        p->year, p->month, p->date);
    printf("Their address : %p %p %p\n", 
        &(p->year), &(p->month), &(p->date));
}

int main(int argc, char ** argv)
{
    Person * p1 = Person_construct(1989,9, 21, "Amy");
    Person_print(p1);

    Person * p3 = p1;
    p3->name = "echo";
    Person_print(p3);
    Person_print(p1); // p1's name should have changed to "echo".
    Person_address_print(p1);
    Person_address_print(p3);


    Person_destruct(p1);
    // p3 is pointing to the same memory of p1, should not destruct(p3)

    return EXIT_SUCCESS;

}

The environment I am running

gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0

valgrind-3.13.0

Thanks so much for any clue!

2
  • 8
    p3->name = "echo"; Why do you do that? That's the likely cause of your problem. It not only causes a memory leak because the original p3->name memory is lost but it also causes a problem when free(p->name) is called as p->name is no longer a pointer to dynamically allocated memory.
    – kaylum
    Commented Dec 26, 2019 at 8:46
  • 2
    @kaylum you are right!! Thanks so much! I wish i could upvote your advice as the final answer! Happy holiday!
    – helloworld
    Commented Dec 26, 2019 at 20:06

1 Answer 1

6

@kaylum 's answer inspires me. I forget C's string literal concept, where if I did p3->name = "echo", I manually let p3->name points to a string literal and its previously pointed char array, which is malloc at heap memory, will be lost. This is the reason I am failed when free(p->name);

Thanks @kaylum !

1
  • 1
    So what did you change afterwards?
    – topkek
    Commented May 1, 2022 at 6:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.