Is this a good practice of freeing dynamically allocated memory or it's not?

234 Views Asked by At

I wrote the following code sample:

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

char *test(void);

int main()
{
    char *base_ptr = NULL;

    base_ptr = test();

    for (char i = 0; i < 5; i++)
    {
        printf("base_ptr[%hhi] = %hhi\n", i, base_ptr[i]);
    }

    free(base_ptr);
    
    return 0;
}

char *test(void)
{
    char *local_ptr = NULL;


    local_ptr = (char *)malloc(5*sizeof(char));

    for (char i = 0; i < 5; i++)
    {
        scanf(" %hhi", &local_ptr[i]);
    }

    return local_ptr;
}

So, I know that once allocated by "malloc()" or "calloc()", I have to free the allocated memory using the "free()" function.

In the code sample I'm showing, I'm doing the allocation in the function "test", which returns a pointer. The pointer returned carries the base address of the allocated array. Inside the function "test()" there is no use of the function "free()", since reaching the return operator, the program leaves the function, which leads to freeing the memory as from the function itself, so from all of it's local variables, including the pointer, which holds the base address.

But inside the function "main()", I'm keeping that address in the pointer "base_ptr". I'm printing all the values, which I assigned in the already terminated function "test()", then I'm freeing the base adddress, using the function "free()".

I have a couple of questions regarding this.

Does this way of freeing alocated memory creates risk of memory leak, is it a good practice at all?

Is freeing dynamically allocated memory via function end or return the same as "free()" function?

If the memory, occupied (and initialized) by the function "test()" is freed due to it's execution end, isn't dangerous to access it's addresses in such maner, as in the code sample?

3

There are 3 best solutions below

0
chux - Reinstate Monica On BEST ANSWER

Is this a good practice of freeing dynamically allocated memory or it's not?

Yes. It is good practice.

Code checked out memory. Put it away when done .

"freed due to it's execution end" is a poor excuse. It is easier to port code to larger tasks that do not rely on that.

test() should clearly document that it is returning allocated memory that later needs a free().


These do not change the above answer, yet the troubles in OP's sample code belie other issues.

Code does have other weak or poor practices:

Lack of error check

Better code tests if malloc() succeeded.

Cast not needed

Castling the return value of malloc() is not needed.

Failure to check the return value of scanf().

Better code checks the return value.

Size to the referenced object, not type

Assign at declaration

//  char *local_ptr = NULL;
// local_ptr = (char *)malloc(5*sizeof(char));

char *local_ptr = malloc(sizeof local_ptr[0] * 5);

Pedantic: Use matchings specifier

%hhi matches a signed char *. It might fail for a char *.

Space is redundant

scanf("%hhi",... performs like scanf(" %hhi",... as %hhi skips leading white-space.

1
0___________ On

But inside the function "main()", I'm keeping that address in the pointer "base_ptr". I'm printing all the values, which I assigned in the already terminated function "test()", then I'm freeing the base adddress, using the function "free()".

Returning from the function does not free the memory allocated via malloc family function. Those memory blocks have a lifetime the same as the program unless you free them yourself. In your function, you do not return a local pointer, only the reference to the allocated memory and this reference is stored in that local pointer.

Automatic storage duration objects instead have a lifetime same as enclosing scope and they stop to exist when you leave this scope

Example:

int *foo(void)
{
    int a[10];
    return a;  //returning pointer to the object which lifetime will end when 
               //function returns. If you use this returned value 
               //it will invoke undefined behaviour
}

Is freeing dynamically allocated memory via function end or return the same as "free()" function?

No, you need to free it yourself by calling free function.

Does this way of freeing alocated memory creates risk of memory leak, is it a good practice at all?

No, there is no risk of using free, and it is a very good practice

If the memory, occupied (and initialized) by the function "test()" is freed due to it's execution end, isn't dangerous to access it's addresses in such maner, as in the code sample?

This memory is not freed when you return from the function and it is valid until you 'free' it yourself.

Modern operating systems free the dynamically allocated memory on program termination, but it a good practice to free them.

0
Harith On

Yes, although most modern operating systems will free the memory on program termination, that isn't an excuse for having memory leaks all over one's code.

Also note that the return value of malloc() and family need not be cast. These functions return a generic pointer type (void *) that is implicitly converted to any other pointer type. As such, it is redundant and only serves to clutter one's code.

#if 0
    char *local_ptr = NULL;
    local_ptr = (char *)malloc(5*sizeof(char));
#else
    char *local_ptr = malloc (5);
    if (!local_ptr) {
        complain();
    }
#endif

We do not need sizeof(char) as it's defined by the standard to be 1 and there is no point in first declaring and initialising the pointer to NULL, only to overwrite it with the return of malloc() immediately.