Freeing specific element from dynamically allocated array of structs

Question

I have a simple struct s_person and constructor(int number) that takes the number of s_person we want to construct and returns a pointer to it.

In this example I have constructed 2 s_person, and assigned values to their attributes, name. And it works.

#include<stdio.h>
#include<stdlib.h>
#define MAX_NAME 50
typedef struct s_person {
    char *name;
    double grade;
}s_person;

s_person* construct(int number){
    int each;
    s_person *person = (s_person*)malloc(sizeof(s_person) * number);
    if(person){
        for(each=0; each<number; each++){
            person[each].name = (char*)malloc(sizeof(char));
        }
    }
    return person;
}

main()
{
    s_person *person = construct(2);
    person[0].name = "steven";
    person[1].name = "michael";

    printf("%s %s\n", person[0].name, person[1].name);

    // This works
    free(&person[0]);
    printf("%s %s\n", person[0].name, person[1].name);

    //This doesn't work
    free(&person[1]); // Crashes here
    printf("%s %s\n", person[0].name, person[1].name);
}

When I try to free the element 0 and print values again it works.

But if I try to free the element 1 and print values it crashes.

So, how can I free memory for the specific element in this array of structs?

And is it a valid way to make dynamically allocated memory for array of structs with constructor(int number) function or is there a better practice?

Each free should match a malloc you cannot free a part of a malloc. You need to do one malloc for each person — Ôrel, May 24 '15 at 10:51

score 1 · Accepted Answer · edited Apr 12 '18 at 08:52

Your program has several errors most of which are related to memory allocation and memory leaks.

For example in function construct you allocte memory for data member name

    for(each=0; each<number; each++){
        person[each].name = (char*)malloc(sizeof(char));

and then in main you overwrite this values

person[0].name = "steven";
person[1].name = "michael";

Thus you have no access any more to the allocated memory and it can not be freed.

As for this statement

free(&person[1]);

then you did not allocate person[1] separatly. You allocated one extent for two instances of the structure. So you may free only this allocated extent.

You could in function construct allocate each instance of the structure separatly by allocating an array of pointers to structures. In this case the function would look the following way

s_person ** construct( int number )
{
    int each;

    s_person **persons =  ( s_person ** ) malloc( number * sizeof( s_person * ) );

    if ( persons != NULL )
    {
        for ( each = 0; each < number; each++ )
        {
            persons[each] = ( s_person * )malloc( sizeof( s_person ) );
            if ( persons[each] != NULL )
            {
                persons[each]->name = ( char* )malloc( MAX_NAME );
            }
        }
    }

    return persons;
}

And in main you have to use standard C function strcpy to initialize data member name of allocated structures with string literals.

#include <string.h>

//...

int main( void )
//^^^^^^^^^^^^^^
{
    s_person **persons = construct( 2 );
    if ( persons[0] ) strcpy( persons[0]->name, "steven" );
    if ( persons[1] ) strcpy( persons[1]->name, "michael" );

    //...

In this case you can delete each separate element. For example

free( persons[0]->name );
free( persons[0] );
persons[0] = NULL;

free( persons[1]->name );
free( persons[1] );
persons[1] = NULL;

// and at last

free( persons );

http://stackoverflow.com/questions/7545365/why-does-this-code-segfault-on-64-bit-architecture-but-work-fine-on-32-bit — Ôrel, May 24 '15 at 11:22
@Ôrel It is what you said and what you referenced to. One very low-qualified programmer wrote a stupidity and other low-qualified programmers repeat it after him. I already down voted that answer. — Vlad from Moscow, May 24 '15 at 11:26
But I want to understand why you cast it and why you over react with this remark. In C you don't need to cast, so why do it ? — Ôrel, May 24 '15 at 11:30
@Ôrel Yes, in C you may omit casting. But if you may something to do this does not mean that you should do this. Casting is a means of documentation of your program. Consider for example p = malloc( sizeof( *p ) ); Can you say what type of obect was allocated? You will need to scroll your program that to find the definition of p and to determine the type. — Vlad from Moscow, May 24 '15 at 11:42
@Ôrel Or imaging that you have two pointers int *p; and int ( *q )[5]; And somewhere in the program there is statement *q = malloc( 10 * sizeof( int ) );Can you say whether it is valid statement and is not a bug? What bug am I saying? For example there is a typo and there must be *p = malloc( 10 * sizeof( int ) ); But by mistake the programmer used q instead of p. It is very difficult to find such bugs. Or imagine you decided to make a C++ program using C code. In this case your life will be too short that to update all calls of malloc that they would be compiled by the C++ compiler. — Vlad from Moscow, May 24 '15 at 11:47

score 0 · Answer 2 · answered May 24 '15 at 10:56

You cannot free a part of a memory block.

Within your function construct you allocate a memory area with the size of sizeof(s_person) * number bytes. If you try to free the first element you free the entire memory (notice that &person[0] is equal to the value returned by construct(2);.

If you try to free &person[1] you try to free an unknown memory area and free(&person[1]) will fail. Freeing an invalid/unknown memory area does not lead to a crash. But in the code you've posted you free the entire memory area of person which leads to an access violation when trying to get the address of person[1] which is the input for your free call.

If you want to free single persons you may want to use linked lists instead of one huge array... There are numerous examples on the web.

I've seen another issue within your code. Looking at your function construct I've noticed that you allocate a single byte for the name of the persone (but I guess you want to have a maximum of MAX_NAME characters for the name). So either modify person[each].name = (char*)malloc(sizeof(char)); to person[each].name = (char*)malloc(sizeof(char) * MAX_NAME); or change your code into the following:

#define MAX_NAME 50
typedef struct s_person {
    char name[MAX_NAME];
    double grade;
}s_person;

s_person* construct(int number){
    s_person *person = (s_person*)malloc(sizeof(s_person) * number);
    return person;
}

I recommend the latter since you cannot forget to free the memory allocated for the name of each person which makes maintaning your code easier.

score 0 · Answer 3 · answered May 24 '15 at 11:11

Why malloc personn you can use the stack:

#include<stdio.h>                                                              
#include<stdlib.h>                                                             
#define MAX_NAME 50                                                            
typedef struct s_person {                                                      
    char *name;                                                                
    double grade;                                                              
}s_person;                                                                     


main()                                                                         
{                                                                              
    s_person person[2];                                                        
    person[0].name = "steven";                                                 
    person[1].name = "michael";                                                

    printf("%s %s\n", person[0].name, person[1].name);                         
}

Freeing specific element from dynamically allocated array of structs

3 Answers3