How to free allocated memory?

[Ning]

Thanks for the replies on my last post
"Must I free allocated memory before the program exits? "
I was aware the danger of memory leak, and you brought up
some excellent points that I wasn't pay much attention to.

The problem is that I find it's hard to write a program with no
memory leak. Like in my course project, I am supposed to
allocate a two dimensional array of doubles. I use a double**
as the pointer pointing to an array of double*'s. Each of the
double* is pointing to an array of doubles. The problem comes
in when I try to free() the allocated. If I had problem when I try
to allocate memory for the array that the double** was
supposed to point to, then double** is uninitialized and I
shouldn't free the double arrays that the fake double* array is
pointing to. Sorry for using "array" loosely, hope you can get
my point. My solution to this is to initialize every pointer to
NULL, so at the end I can see which pointer is never malloc'ed
or malloc() failed. Moreover, if I call free(NULL), the behavior is
not undefined. So my program looks like this:

#include <stdio.h>
#include <stdlib.h>
#define N 10

int main()
{
int i;
/* initialize pp */
double **pp = NULL;
int ret_value = 0;

/* allocate a NxN array */
pp = malloc(N * sizeof(*pp));
if ( !pp )
goto malloc_fail;

/* initialize pp[n] */
for (i=0; i<N; ++i)
pp = NULL;
for (i=0; i<N; ++i)
{
pp = malloc(N * sizeof(**pp));
if ( !pp )
goto malloc_fail;
}

{
/* use the array here */
/* hopefully pointers are not screwed up */
}

main_exit:
/* pp != NULL? */
/* If not, pp's are garbage */
/* don't free them. */
if ( pp != NULL )
for (i=0; i<N; ++i)
free(pp);
free(pp);
return (ret_value);

malloc_fail:
ret_value = 1;
fflush(NULL);
fprintf(stderr, "malloc() fails.\n");
fflush(stderr);
goto main_exit;
}

Is there any problem in my code? Is there a
somewhat standard way to handle situations like this? Initialize every
pointer to NULL is not hard to remember, but it is just so easy to
forget.


Thanks.

 

[David Rubin]

The problem is that I find it's hard to write a program with no
memory leak. Like in my course project, I am supposed to
allocate a two dimensional array of doubles. I use a double**
as the pointer pointing to an array of double*'s. Each of the
double* is pointing to an array of doubles. The problem comes
in when I try to free() the allocated. If I had problem when I try
to allocate memory for the array that the double** was
supposed to point to, then double** is uninitialized and I
shouldn't free the double arrays that the fake double* array is
pointing to. Sorry for using "array" loosely, hope you can get
my point. My solution to this is to initialize every pointer to
NULL, so at the end I can see which pointer is never malloc'ed
or malloc() failed. Moreover, if I call free(NULL), the behavior is
not undefined.

You are wrong on this account: free(NULL) *is* defined, and it's
perfectly legal and valid.

So my program looks like this:

#include <stdio.h>
#include <stdlib.h>
#define N 10

int main()

int main(void)

{
int i;
/* initialize pp */
double **pp = NULL;

Don't need to use this initialization.

int ret_value = 0;

/* allocate a NxN array */
pp = malloc(N * sizeof(*pp));
if ( !pp )
goto malloc_fail;

How about

if(pp == NULL){
perror("malloc");
exit(EXIT_FAILURE);
}

/* initialize pp[n] */
for (i=0; i<N; ++i)
pp = NULL;

Don't need the above loop.

for (i=0; i<N; ++i)
{
pp = malloc(N * sizeof(**pp));
if ( !pp )
goto malloc_fail;

if(pp == NULL){
perror("malloc");
while(i >= 0){
free(pp[i--]);
}
exit(EXIT_FAILURE);
}

}

{
/* use the array here */
/* hopefully pointers are not screwed up */
}

[snip - labels]
Here, you can let the OS clean up after you, or you can use

for(i=0; i < N; ++i){
free(pp);
}
free(pp);

}

Is there any problem in my code? Is there a
somewhat standard way to handle situations like this? Initialize every
pointer to NULL is not hard to remember, but it is just so easy to
forget.

You don't have to initialize the pointers to NULL. You could simplify
the code with a macro (or function) which deletes the array:

/* free array T **p with secondary size n */
#define afree(p,n) do{while(--n >= 0) free(p[n]); free(p)}while(0)

N.B. You have to be careful with your "n" argument when using the macro
in an error leg. A function takes T ***p and requires another level of
derefencing.

HTH,

/david

 

[Mike Wahler]

Thanks for the replies on my last post
"Must I free allocated memory before the program exits? "
I was aware the danger of memory leak, and you brought up
some excellent points that I wasn't pay much attention to.

Which is why it's a good 'general rule' to always
free what you malloc or realloc.

The problem is that I find it's hard to write a program with no
memory leak. Like in my course project, I am supposed to
allocate a two dimensional array of doubles. I use a double**
as the pointer pointing to an array of double*'s. Each of the
double* is pointing to an array of doubles. The problem comes
in when I try to free() the allocated. If I had problem when I try
to allocate memory for the array that the double** was
supposed to point to, then double** is uninitialized and I
shouldn't free the double arrays that the fake double* array is
pointing to.

If 'malloc()' fails, it returns NULL. It's safe to pass
NULL as the argument to 'free()', it results in a
"do-nothing" operation.

Sorry for using "array" loosely, hope you can get
my point. My solution to this is to initialize every pointer to
NULL, so at the end I can see which pointer is never malloc'ed
or malloc() failed.

Not necessary. free(NULL) is perfectly safe.

Moreover, if I call free(NULL), the behavior is
not undefined.

Right.

More below, after your code.

So my program looks like this:

#include <stdio.h>
#include <stdlib.h>
#define N 10

int main()
{
int i;
/* initialize pp */
double **pp = NULL;
int ret_value = 0;

/* allocate a NxN array */
pp = malloc(N * sizeof(*pp));
if ( !pp )
goto malloc_fail;

/* initialize pp[n] */
for (i=0; i<N; ++i)
pp = NULL;
for (i=0; i<N; ++i)
{
pp = malloc(N * sizeof(**pp));
if ( !pp )
goto malloc_fail;
}

{
/* use the array here */
/* hopefully pointers are not screwed up */
}

main_exit:
/* pp != NULL? */
/* If not, pp's are garbage */
/* don't free them. */
if ( pp != NULL )
for (i=0; i<N; ++i)
free(pp);
free(pp);
return (ret_value);

malloc_fail:
ret_value = 1;
fflush(NULL);
fprintf(stderr, "malloc() fails.\n");
fflush(stderr);
goto main_exit;
}

Is there any problem in my code? Is there a
somewhat standard way to handle situations like this? Initialize every
pointer to NULL is not hard to remember, but it is just so easy to
forget.

And unnecessary. I do think too many things are
cluttering up your 'main', though. And I hate those
'goto's.

Encapsulate!

In my example below, note how all the 'messy details'
are factored out into separate functions, and 'main()'s
flow of execution is much easier to follow.

Also, this 'refactoring' greatly eases debugging
by making it much easier to isolate a problem.
Much better to find out quickly that there's a
bug in a specific small function, than 'somewhere'
in a large function.

Example:

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

double **alloc2d(size_t rows, size_t cols)
{
double **result;
size_t row = 0;
size_t col = 0;

if(result = malloc(rows * sizeof *result))
{
if(*result = malloc(rows * cols * sizeof **result))
for(row = 1; row < rows; ++row)
result[row] = *result + row * cols;
else
{
free(result);
result = NULL;
}
}

return result;
}

void release2d(double **arr)
{
free(*arr);
free(arr);
}

void populate2d(double **arr, size_t rows, size_t cols)
{
size_t row = 0;
size_t col = 0;

for(row = 0; row < rows; ++row)
for(col = 0; col < cols; ++col)
arr[row][col] = row * cols + col;
}

void show2d(double **arr, size_t rows, size_t cols)
{
size_t row = 0;
size_t col = 0;

for(row = 0; row < rows; ++row)
{
for(col = 0; col < cols; ++col)
printf("%4.0f", arr[row][col]);

putchar('\n');
}
}

int main()
{
size_t rows = 10;
size_t cols = 10;
size_t row = 0;
size_t col = 0;
double **arr = NULL;
const int retcodes[] = {EXIT_FAILURE, EXIT_SUCCESS};
int status = 0;

/* everything about allocating, using, and releasing
the array memory is expressed in this simple loop: */
if(status = (arr = alloc2d(rows, cols)) != NULL) /* allocate */
{
populate2d(arr, rows, cols); /* manipulate */
show2d(arr, rows, cols); /* manipulate */
release2d(arr); /* release */
}

/* tell user if something went wrong */
printf("%s\n", status ? "" : "Cannot allocate memory");

/* tell calling environment if something went wrong */
return retcodes[status];
}

HTH,
-Mike

 

[CBFalconer]

Thanks for the replies on my last post
"Must I free allocated memory before the program exits? "
I was aware the danger of memory leak, and you brought up
some excellent points that I wasn't pay much attention to.

The problem is that I find it's hard to write a program with no
memory leak. Like in my course project, I am supposed to
allocate a two dimensional array of doubles. I use a double**
as the pointer pointing to an array of double*'s. Each of the
double* is pointing to an array of doubles. The problem comes

.... snip ...

No it isn't. A double * is a pointer to double. A double ** is a
pointer to a pointer to double. There is no array there.

Don't start a new thread every time you post. Make a reply to the
appropriate article, snip the immaterial stuff, and append your
answer at the end (or intermixed).

 

[Ning]

You are wrong on this account: free(NULL) *is* defined, and it's
perfectly legal and valid.

Yeah, I was not precise. I should have said free(NULL) is
well defined.

int main(void)

Is " int main() " leagal in C89?

Don't need the above loop.

for (i=0; i<N; ++i)
{
pp = malloc(N * sizeof(**pp));
if ( !pp )
goto malloc_fail;

if(pp == NULL){
perror("malloc");
while(i >= 0){
free(pp[i--]);
}
exit(EXIT_FAILURE);

Thanks for this piece of code. I didn't think of doing it this way.
But I think it may be better this way, in case I change i from int
to size_t

do{
free(pp[i--]);
} while ( i > 0);

 

[Ning]

... snip ...
No it isn't. A double * is a pointer to double. A double ** is a
pointer to a pointer to double. There is no array there.

Sorry, I used the term "array" loosely. What I meant was the double*
is pointing to the fist unit of memory allocated for doubles.

Don't start a new thread every time you post. Make a reply to the
appropriate article, snip the immaterial stuff, and append your
answer at the end (or intermixed).

I just thought this post is not related to the last thread that much, so I
started a new one. Sorry if I messed up the board.

 

[Ning]

Encapsulate!

In my example below, note how all the 'messy details'
are factored out into separate functions, and 'main()'s
flow of execution is much easier to follow.

Thank you. Your code really answered a lot of questions that
have been bugging me for a while.

 

[Mike Wahler]

[snip]

/* everything about allocating, using, and releasing
the array memory is expressed in this simple loop: */
if(status = (arr = alloc2d(rows, cols)) != NULL) /* allocate */
{
populate2d(arr, rows, cols); /* manipulate */
show2d(arr, rows, cols); /* manipulate */
release2d(arr); /* release */
}

Don't know what I was thinking, that's not a loop,
just an 'if' statement. Sorry about that.

-Mike

/* tell user if something went wrong */
printf("%s\n", status ? "" : "Cannot allocate memory");

/* tell calling environment if something went wrong */
return retcodes[status];
}

HTH,
-Mike

 

[David Rubin]

Ning wrote:

[snip]

if(pp == NULL){
perror("malloc");
while(i >= 0){
free(pp[i--]);
}
exit(EXIT_FAILURE);

Thanks for this piece of code. I didn't think of doing it this way.
But I think it may be better this way, in case I change i from int
to size_t

do{
free(pp[i--]);
} while ( i > 0);

You have to also free(pp[0])! But, you are correct about the lurking
error. So, you may want either a for-loop and an extra variable, or an
extra free statement after the while loop.

/david

 

[Allin Cottrell]

^^^^^^^^^^^^^^^^

You are wrong on this account: free(NULL) *is* defined, and it's
perfectly legal and valid.

That is, Ning is in fact perfectly correct on this account!