Malloc and free questions - learner questions

P

pkirk25

If I start a function with this:
char *temp_string = malloc(MAXLEN);
char *item_id_string = malloc(MAXLEN);
char *item_description_string = malloc(MAXLEN);

Can I assume that malloc does exactly as its told or so I need to
check?

When the function finishes, it the memory freed up or do I need to
explicitly call free(temp_string) at end and free(item_id_string)
free(item_description_string ) after I am finished with them?

I can tell these are newbie type questions but K&R is very terse on
this subject.
 
J

Joel Haugen

pkirk25 said:
If I start a function with this:
char *temp_string = malloc(MAXLEN);
char *item_id_string = malloc(MAXLEN);
char *item_description_string = malloc(MAXLEN);

Can I assume that malloc does exactly as its told or so I need to
check?

When the function finishes, it the memory freed up or do I need to
explicitly call free(temp_string) at end and free(item_id_string)
free(item_description_string ) after I am finished with them?

I can tell these are newbie type questions but K&R is very terse on
this subject.
For every malloc() call, there should be a free() call. There isn't any
automatic garbage collection.
 
C

Cong Wang

pkirk25 said:
If I start a function with this:
char *temp_string = malloc(MAXLEN);
char *item_id_string = malloc(MAXLEN);
char *item_description_string = malloc(MAXLEN);

Can I assume that malloc does exactly as its told or so I need to
check?

In most cases, malloc() will do what you tell it. But it's still very
necessary to check whether the return value of it is NULL.
When the function finishes, it the memory freed up or do I need to
explicitly call free(temp_string) at end and free(item_id_string)
free(item_description_string ) after I am finished with them?

You must call free() by yourself. If you forget that, memory leak will
occur.
 
A

Andrea Laforgia

If I start a function with this:
char *temp_string = malloc(MAXLEN);
char *item_id_string = malloc(MAXLEN);
char *item_description_string = malloc(MAXLEN);

Can I assume that malloc does exactly as its told or so I need to
check?

You must always check whether malloc() succeeded or not by examining
its return value: if 0 (NULL) it failed. If malloc() succeeds, then
the size of the memory allocated is exactly what you specified.
When the function finishes, it the memory freed up or do I need to
explicitly call free(temp_string) at end and free(item_id_string)
free(item_description_string ) after I am finished with them?

Usually, there's no built-in garbage collector, with C (and C++)
compilers. Although almost all operating systems automatically free
memory allocated by programs, you need to explicitly free() it, in
order to use memory in a rational way and to avoid leaks.
 
C

clayne

pkirk25 said:
If I start a function with this:
char *temp_string = malloc(MAXLEN);
char *item_id_string = malloc(MAXLEN);
char *item_description_string = malloc(MAXLEN);

Can I assume that malloc does exactly as its told or so I need to
check?

No, you need to check.

assert((temp_string = malloc(MAXLEN)));
assert((item_id_string = malloc(MAXLEN)));
assert((item_description = malloc(MAXLEN)));

Or write error checking wrapper functions or macros. It's also
generally considered bad style to call functions in the initial
declaration as it removes intent or prior use from surrounding context
- not to mention it's setting oneself up for a leak if you return
*early* due to function logic, error, etc. and don't exercise proper
discipline by free()ing before return. There's no need to pre-allocate
if you aren't going to even use it yet.
When the function finishes, it the memory freed up or do I need to
explicitly call free(temp_string) at end and free(item_id_string)
free(item_description_string ) after I am finished with them?

You must call free() once, and only once, for each object freed.
 
A

Andrew Poelstra

No, you need to check.

assert((temp_string = malloc(MAXLEN)));
assert((item_id_string = malloc(MAXLEN)));
assert((item_description = malloc(MAXLEN)));
only once, for each object freed.

And when it's time to ship and you turn off debug mode, you just assume
that your customers are using a separate and infinite universe as a
memory bank?

/Never/ use assert()'s for resource-checking.
 
B

Bill Pursell

pkirk25 said:
If I start a function with this:
char *temp_string = malloc(MAXLEN);
char *item_id_string = malloc(MAXLEN);
char *item_description_string = malloc(MAXLEN);

Can I assume that malloc does exactly as its told or so I need to
check?

As others have pointed out, you cannot assume that malloc succeeds,
and must check if any of the returns are NULL. I find a lot of code
that does this the annoying way:

if ( ( a = malloc(MAXLEN)) == NULL) {
fprintf(stderr, "Not enought space for a!\n");
exit(-1);
}

Don't do that. A few nicer methods are:

A wrapper function, if simply aborting is okay:

void *
xmalloc(size_t size)
{
void * ret;
if ( (ret = malloc(size)) == NULL) {
fprintf(stderr, "Out of Memory\n");
exit ( EXIT_FAILURE);
}
return ret;
}

and checking mulitple calls once:
a=malloc(M);
b=malloc(M);
c=malloc(2*M);
if ( a == NULL || b == NULL || c == NULL) {
/* handle the error */
}

Another nice convenience function is the one which looks like:
error_t my_malloc(void *a, size_t, ...);
You pass it an arbitrary number of arguments and if it returns
0, then you know all of them have been allocated. If it returns
non-zero, then none of them have been and any memory that
may have been allocated has already been freed. This way,
you can write:

char *a, *b, *c, *d;
if (my_malloc( &a, M, &b, N , &c, M*N , &d, M+N, NULL)) {
/* handle error*/
}

Instead of the much uglier:

if ( ( a = malloc(M)) == NULL) {
handle_error();
}
else if ( (b = malloc(N)) == NULL) {
free(a);
handle_error();
}
etc..
 
R

Richard Heathfield

pkirk25 said:
If I start a function with this:
char *temp_string = malloc(MAXLEN);
char *item_id_string = malloc(MAXLEN);
char *item_description_string = malloc(MAXLEN);

Can I assume that malloc does exactly as its told or so I need to
check?

You can assume it will try, but it may not succeed, so look for NULL
returns.
When the function finishes, it the memory freed up

No, and indeed that's the whole point (or at least a big part of it). Call
free() when you've finished with the memory. In the meantime, don't forget
where it is. (Keep a pointer pointing at it.)
 
C

clayne

Andrew said:
And when it's time to ship and you turn off debug mode, you just assume
that your customers are using a separate and infinite universe as a
memory bank?

/Never/ use assert()'s for resource-checking.

No. /Never/ use assert()'s for resource-checking in production code.

But for a toy program that it is in development phase without the
proper error-checking/handling framework in place, assert() is
completely fine. I wasn't going to write hand-holding error checking
functions for the guy, let's be realistic.
 
B

Bill Pursell

clayne said:
No. /Never/ use assert()'s for resource-checking in production code.

But for a toy program that it is in development phase without the
proper error-checking/handling framework in place, assert() is
completely fine. I wasn't going to write hand-holding error checking
functions for the guy, let's be realistic.

I disagree. Assert has an incredibly valuable purpose, and by
demonstrating such a heinous mis-use, you demonstrate bad
techniques to the novice.
 
R

Richard Heathfield

clayne said:
No, you need to check.
Yes.


assert((temp_string = malloc(MAXLEN)));

But not like that.

When you define NDEBUG, it doesn't just turn off the checking. It even turns
off the allocation! Never use assert for this.

assert((item_id_string = malloc(MAXLEN)));
assert((item_description = malloc(MAXLEN)));

Or write error checking wrapper functions or macros.

No, don't do that. Do this:

if(ptr != NULL)
{
do your thing
}
else
{
handle the error in some appropriate way
}

Your way, it's either inconvenient or impossible to "handle the error in
some appropriate way", depending on how you do the wrapping.
It's also
generally considered bad style to call functions in the initial
declaration as it removes intent or prior use from surrounding context

I disagree that this is "generally considered bad style".
- not to mention it's setting oneself up for a leak if you return
*early* due to function logic, error, etc.

and don't exercise proper
discipline by free()ing before return.

If malloc followed that "proper discipline", it would be a singularly
useless function.
There's no need to pre-allocate
if you aren't going to even use it yet.

An emergency reserve can be useful.
You must call free() once, and only once, for each object freed.

No, you must call free() exactly once for each block of memory allocated to
your program by calloc(), malloc(), or realloc() (except when realloc()
cleans up for you on re-sizing, obviously).
 
R

Richard Heathfield

Bill Pursell said:
As others have pointed out, you cannot assume that malloc succeeds,
and must check if any of the returns are NULL. I find a lot of code
that does this the annoying way:

if ( ( a = malloc(MAXLEN)) == NULL) {
fprintf(stderr, "Not enought space for a!\n");
exit(-1);
}

Don't do that.

Agreed. If you must use exit(), pass it EXIT_FAILURE. But better still, try
to recover from the failure. Oh, and that message sucks. How is the user
supposed to know what

Not enought space for a!

means?
A few nicer methods are:

A wrapper function, if simply aborting is okay:

That's pretty unlikely except in "one-offs" and student code. If your
favourite word processor simply stopped executing on running out of memory
(and never mind that 30 minutes worth of unsaved typing you've just done),
it wouldn't remain your favourite word processor for long.
Another nice convenience function is the one which looks like:
error_t my_malloc(void *a, size_t, ...);
You pass it an arbitrary number of arguments and if it returns
0, then you know all of them have been allocated. If it returns
non-zero, then none of them have been and any memory that
may have been allocated has already been freed. This way,
you can write:

char *a, *b, *c, *d;
if (my_malloc( &a, M, &b, N , &c, M*N , &d, M+N, NULL)) {
/* handle error*/
}

Problem: I don't want a char *. I want a T *. Or maybe a T2 *.
 
C

clayne

Bill said:
I disagree. Assert has an incredibly valuable purpose, and by
demonstrating such a heinous mis-use, you demonstrate bad
techniques to the novice.

As rheathfield pointed out, NDEBUG will remove the body generated by
the macro. So yes, not for use in a production environment. My example
was primarily intended as a trite "check for NULL somehow," however
heinous it is (I'm not going to write error checking frameworks for a
newbie, and I pointed him to considering error checking functions or
macros).

I don't even use assert in my own "done" code - I use my own wrappers
just like anyone else.
 
B

Bill Pursell

Richard said:
Bill Pursell said:

Agreed. If you must use exit(), pass it EXIT_FAILURE. But better still, try
to recover from the failure. Oh, and that message sucks. How is the user
supposed to know what

Not enought space for a!

means?

I don't know! When I first started seeing code with messages like
this, I chuckled. Then I saw it some more and started to be
annoyed. Now I just accept it as a fact of life. Fortunately,
code with these message tends to come in code with a clearly
defined set of bad practices, so I can take it as a flag to
warn me to look for certain idiosyncracies.
Problem: I don't want a char *. I want a T *. Or maybe a T2 *.

Why is that a problem?

T *a;
if ( my_malloc( &a, 56 * sizeof *a, NULL) !=0) {
/* handle error */
} else {
/* a now holds 56 T's, if each is a bird, you've got a nice ride :)
*/
}
 
R

Richard Heathfield

clayne said:
As rheathfield pointed out, NDEBUG will remove the body generated by
the macro. So yes, not for use in a production environment. My example
was primarily intended as a trite "check for NULL somehow,"

In future, then, if you mean "check for NULL somehow", please write "check
for NULL somehow", rather than "assert((p = malloc(n)) != NULL);" as it
will save us from going through the whole "what assert is for" debate all
over again.
however
heinous it is (I'm not going to write error checking frameworks for a
newbie, and I pointed him to considering error checking functions or
macros).

I have a number of years experience as a C programmer, so I guess it's
reasonable for me to claim that I'm no newbie - and I don't immediately see
how you would implement a useful set of error-checking functions or macros,
let alone how a newbie could be expected to do so.
I don't even use assert in my own "done" code

Then it's probably a bad idea to advise others on how to use it.
- I use my own wrappers just like anyone else.

I would be curious to see these wrappers, since it seems to me that they
must either be very complicated to use, or completely useless.
 
C

Cong Wang

Bill said:
As others have pointed out, you cannot assume that malloc succeeds,
and must check if any of the returns are NULL. I find a lot of code
that does this the annoying way:

if ( ( a = malloc(MAXLEN)) == NULL) {
fprintf(stderr, "Not enought space for a!\n");
exit(-1);
}

Don't do that. A few nicer methods are:

A wrapper function, if simply aborting is okay:

void *
xmalloc(size_t size)
{
void * ret;
if ( (ret = malloc(size)) == NULL) {
fprintf(stderr, "Out of Memory\n");
exit ( EXIT_FAILURE);
}
return ret;
}

and checking mulitple calls once:
a=malloc(M);
b=malloc(M);
c=malloc(2*M);
if ( a == NULL || b == NULL || c == NULL) {
/* handle the error */
}

I don't think this is better. ;-(
Another nice convenience function is the one which looks like:
error_t my_malloc(void *a, size_t, ...);
You pass it an arbitrary number of arguments and if it returns
0, then you know all of them have been allocated. If it returns
non-zero, then none of them have been and any memory that
may have been allocated has already been freed. This way,
you can write:

char *a, *b, *c, *d;
if (my_malloc( &a, M, &b, N , &c, M*N , &d, M+N, NULL)) {
/* handle error*/
}

Instead of the much uglier:

if ( ( a = malloc(M)) == NULL) {
handle_error();
}
else if ( (b = malloc(N)) == NULL) {
free(a);
handle_error();
}
etc..

On Linux, there is a good function named alloca(), which is like
malloc(), but memory allocated will be automatically freed when the
function that called alloca() returns to its caller.
 
R

Richard Heathfield

Bill Pursell said:
I don't know! When I first started seeing code with messages like
this, I chuckled. Then I saw it some more and started to be
annoyed. Now I just accept it as a fact of life. Fortunately,
code with these message tends to come in code with a clearly
defined set of bad practices, so I can take it as a flag to
warn me to look for certain idiosyncracies.

Why is that a problem?

T *a;
if ( my_malloc( &a, 56 * sizeof *a, NULL) !=0) {
/* handle error */
} else {
/* a now holds 56 T's, if each is a bird, you've got a nice ride :)

a has type T *.
So &a has type T **.

You pick it up as a void *, right?

Okay, so where do you go from there? You can't deref it, because if you have
void *ptr, you can't do *ptr. And if you do this instead: void **ptr; you
lose the automatic conversion.

In short, I'd be fascinated to see the source to my_malloc. :)
 
A

Antti-Juhani Kaijanaho

Cong Wang said:
On Linux, there is a good function named alloca(), which is like
malloc(), but memory allocated will be automatically freed when the
function that called alloca() returns to its caller.

You might want to check the FAQ 7.32, just posted to the group.
 
C

clayne

Cong said:
On Linux, there is a good function named alloca(), which is like
malloc(), but memory allocated will be automatically freed when the
function that called alloca() returns to its caller.

Good luck, you're going to get slammed for mentioning alloca() just as
I mentioned assert().
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,731
Messages
2,569,432
Members
44,832
Latest member
GlennSmall

Latest Threads

Top