should I free mem in function after alloc and returning pointer... ?

[beetle]

Hello,

My question; Should I always free memory previously allocated in a
function even if I returned the pointer to that mem previously ?

For example:

static char *dup(const char *s) {
char *result = (char *)malloc(strlen(s) + 1);

if(result == NULL)
return NULL;

strncpy(result, s, strlen(s));
return result;
// **** Should I free(result) here ?? ****
}

Normally I would reason.. and say yes, but I am not sure... If someone
could ellaborate a bit on this I would be listening..

Thnkx chrs.

 

[Ben Pfaff]

My question; Should I always free memory previously allocated in a
function even if I returned the pointer to that mem previously ?

Nothing after a `return' will be executed, so what you do "after
a `return'" is irrelevant.

 

[David REsnick]

Hello,

My question; Should I always free memory previously allocated in a
function even if I returned the pointer to that mem previously ?

For example:

static char *dup(const char *s) {
char *result = (char *)malloc(strlen(s) + 1);

if(result == NULL)
return NULL;

strncpy(result, s, strlen(s));
return result;
// **** Should I free(result) here ?? ****
}

Normally I would reason.. and say yes, but I am not sure... If someone
could ellaborate a bit on this I would be listening..

Thnkx chrs.

Returning a string to a caller is usually done in 3 ways:
1) return malloc'd string, caller must free
2) return pointer to static storage (usually not reentrant)
3) put string into storage provided by caller

You are doing option 1. If memory is acquired via malloc
it must eventually be freed or it will leak. Typically by
the caller. Your comment about freeing it after the return
is confusing.

A few other random things:

With the odd exception, don't cast malloc return.

You should use strcpy -- you know there is enough room in the malloc'd
buffer, and your use of strncpy will result in a string that is not
null terminated, you are copying one byte too few.

Unless using a C99 compiler, usually best to avoid // comments for
better portability. Others may disagree there. But hey, if you
don't want
int foo = 10 //*how funky*/ MY_CONST;
to work, whatever.

-David

 

[beetle]

Returning a string to a caller is usually done in 3 ways:
1) return malloc'd string, caller must free
2) return pointer to static storage (usually not reentrant)
3) put string into storage provided by caller

You are doing option 1. If memory is acquired via malloc
it must eventually be freed or it will leak. Typically by
the caller. Your comment about freeing it after the return
is confusing.

A few other random things:

With the odd exception, don't cast malloc return.

You should use strcpy -- you know there is enough room in the malloc'd
buffer, and your use of strncpy will result in a string that is not
null terminated, you are copying one byte too few.

Unless using a C99 compiler, usually best to avoid // comments for
better portability. Others may disagree there. But hey, if you
don't want
int foo = 10 //*how funky*/ MY_CONST;
to work, whatever.

-David

Roger.. thnkx...!!

Beetle..

 

[Mike Wahler]

Hello,

My question; Should I always free memory previously allocated in a
function even if I returned the pointer to that mem previously ?

You should free allocated memory when your program
no longer needs it, regardless of scope. However,
imo it's much easier to maintain code that keeps
the allocate-deallocate pairs in the same scope.
Access to allocated memory can be shared among
functions by passing its address as an argument
or return value.

If you assign the address of allocated memory to
a pointer already pointing at some other allocated
memory (and haven't saved a copy of that original
pointer), you'll produce a 'memory leak'. If you
try to free (or access) an already freed pointer,
you'll produce 'undefined behavior'. Freeing
a null pointer will have no effect, i.e. it's
'safe'.

BTW don't cast the return value from malloc().
See the C FAQ for details.

-Mike

 

[Jonathan Bartlett]

1) Nothing after a return is ever executed

2) You should never ever free something that is in use (so if you put a
pointer to something somewhere you SHOULD NOT free the memory)

3) You need to have a memory management policy. You need to decide who
frees what when, and how they know whether or not it can be freed.

Optionally, you can skip this and use a memory manager. Some other
options are available in a DeveloperWorks article I wrote a while ago:

http://www.ibm.com/developerworks/linux/library/l-memory/

Jon

 

[Jonathan Burd]

Hello,

My question; Should I always free memory previously allocated in a
function even if I returned the pointer to that mem previously ?

For example:

static char *dup(const char *s) {
char *result = (char *)malloc(strlen(s) + 1);

Cast malloc only if you know what you are doing.

It is not clear from the context whether you have
included the stdlib.h header file.
If you don't include the stdlib.h file, the prototype
is not declared. malloc() would, in that case,
implicitly return an ``int" (C'89). ints and pointers
may not have the same size on some platforms.
On the DEC Alpha, for example, an int is 4 bytes
and a pointer is 8 bytes.

By casting malloc, you are effectively saying
"I know what I am doing. I want the int
returned by malloc to be treated as a char*."

Not casting it when the prototype isn't declared
should also make the compiler warn you that you
are making a pointer from an int without casting.
You get an opportunity to track the bug!

<snip>

Include the prototype and don't cast malloc
unless you really need to.

Regards,
Jonathan.

 

[Keith Thompson]

With the odd exception, don't cast malloc return.

The "odd exception" you're referring to is writing code that needs to
compile as both C and C++, right?

Warning: Unrealistic speculation follows.

The only case I can think of in pure C in which it makes sense to cast
the result of malloc() is passing the result directly to a function
with a "..." prototype, when the corresponding va_arg() invocation
inside the function expects a pointer type other than void* (or a
pointer-to-character type). But even then, it almost certainly makes
more sense to assign the result of malloc() to a variable of the
appropriate type (without a cast) and pass the variable to the
function.

The same issue could show up with a call to a function with an
old-style (non-prototype) declaration, but that's equally unlikely.

 

[David Resnick]

The "odd exception" you're referring to is writing code that needs to
compile as both C and C++, right?

Yes. I could have also said "unless your initials are P.J.P.", as he
seems to be the only one on the group who has expressed a real need
for that.

-David

 

[Chris Croughton]

Yes. I could have also said "unless your initials are P.J.P.", as he
seems to be the only one on the group who has expressed a real need
for that.

He isn't, I've had the same requirement (maintaining two pieces of code,
one for C and the other for C++, is far more risky than having an
explicit cast where it isn't required). There are other exceptions:

When external coding standards require it.

When using a compiler which exercises its standard-given right to give
diagnostics about anything it feels like it, and company policy is to
release no code with warnings.

The same for other casts (assigning an int to an enum or to a smaller
integer variable is allowed by the standard, but at least one compiler
complains if there isn't an explicit cast).

Chris C

 

[Keith Thompson]

[...]

With the odd exception, don't cast malloc return.

The "odd exception" you're referring to is writing code that needs to
compile as both C and C++, right?

Yes. I could have also said "unless your initials are P.J.P.", as he
seems to be the only one on the group who has expressed a real need
for that.

He isn't, I've had the same requirement (maintaining two pieces of code,
one for C and the other for C++, is far more risky than having an
explicit cast where it isn't required).

Ok. I would think that writing the code in pure C, and using extern
"C" when calling it from C++, would be a better solution, but there
could certainly be legitimate reasons for doing it that way.

There are other exceptions:

When external coding standards require it.

Are there actual C coding standards that require casting malloc()?
Unless it's specifically for the purpose of compiling the code as both
C and C++, that's just ridiculous.

I suppose a coding standard could ban the use of any parentheses in
expressions unless they're strictly necessary to override the operator
precedence rules.

When using a compiler which exercises its standard-given right to give
diagnostics about anything it feels like it, and company policy is to
release no code with warnings.

The same for other casts (assigning an int to an enum or to a smaller
integer variable is allowed by the standard, but at least one compiler
complains if there isn't an explicit cast).

Again, management can force programmers to write bad code if they want
to. I'm glad I don't have to deal with that kind of nonsense.

A more reasonable policy might be to require a comment for any source
construct that triggers a warning, explaining why it's not a problem
in this case.

 

[Chris Croughton]

Ok. I would think that writing the code in pure C, and using extern
"C" when calling it from C++, would be a better solution, but there
could certainly be legitimate reasons for doing it that way.

An example is when a function needs overloading in the C++ code, it
isn't allowed to overload something declared with extern "C". For
example, when a signature with const as well as non-const versions of a
parameter is wanted (as with some of the string functions).

Are there actual C coding standards that require casting malloc()?
Unless it's specifically for the purpose of compiling the code as both
C and C++, that's just ridiculous.

Yes, it makes it clearer in the view of a lot of programmers (and yes
there are also a lot of programmers who think it makes it more obscure,
and both groups will say that the other is 'wrong').

I suppose a coding standard could ban the use of any parentheses in
expressions unless they're strictly necessary to override the operator
precedence rules.

I've seen that, yes.

Again, management can force programmers to write bad code if they want
to. I'm glad I don't have to deal with that kind of nonsense.

It's not bad code, it's self-documenting code. If I see:

fred = (char)bill;

then I know that the value of bill is being coerced to fit in a char,
without having to wonder whether fred is a char or an int. Having found
a number of bugs caused by someone declaring a variable of the wrong
type (I recently couldn't understand why a buffer wasn't being input,
until I found that my "convenient size" of 256 was being put into an
unsigned char and being silently truncated to zero!) I would rather see
explicit casts where truncation occurs.

A more reasonable policy might be to require a comment for any source
construct that triggers a warning, explaining why it's not a problem
in this case.

Not sensible with automated build tools which report warnings and errors
if someone has to go through the code every day (several megabytes of
it) to find out whether the thousands of warnings are 'real' or not.
Not sensible when it would need a comment on every other line saying
"the following line gives a warning with the ARM compiler, ignore it".
I've worked in several production environments where QA refuse to
release code which produces warnings -- and since other procedures say
that code can't be checked in without review of the changes programmers
can't get away with just inserting casts to "shut the compiler up"
without being questioned why the warning was happening and whether the
value truncation was valid.

Chris C

 

[Keith Thompson]

On Wed, 26 Jan 2005 19:37:51 GMT, Keith Thompson


Yes, it makes it clearer in the view of a lot of programmers (and yes
there are also a lot of programmers who think it makes it more obscure,
and both groups will say that the other is 'wrong').

The difference is that the programmers who think casting malloc() is a
good idea (except for the rare C/C++ compatibility requirement) really
are wrong. (Note the lack of a smiley here.)

I've seen that, yes.

Seriously? I meant that as a joke.

Ok, how about this: a coding standard that requires all identifiers to
be of the form id_NNNN, where NNNN is a decimal number, and a
separately maintained table describes what each one is used for.

If an imposed coding standard is sufficiently stupid, you as a
programmer need to say so, and if management insists on enforcing it,
they need to be told what it's going to cost in productivity.

[...]

It's not bad code, it's self-documenting code. If I see:

fred = (char)bill;

then I know that the value of bill is being coerced to fit in a char,
without having to wonder whether fred is a char or an int.

It tells you the value of bill is being coerced to fit in a char, but
it doesn't tell you the type of either bill or fred. If you change
fred from char to short, do you go through the code and change all the
casts? Keep in mind that the compiler won't complain about
fred = (char)bill;
even if fred is a short. For example, it might silently truncate the
32-bit value of bill to 8 bits before assigning it to the 16-bit
object fred.

Having found a number of bugs caused by someone declaring a variable
of the wrong type (I recently couldn't understand why a buffer
wasn't being input, until I found that my "convenient size" of 256
was being put into an unsigned char and being silently truncated to
zero!) I would rather see explicit casts where truncation occurs.

I would rather not see explicit casts unless they're actually
necessary. You can use a cast to document the type of an expression,
but that's really an abuse of the constuct. In the above assignment
what *you* mean by the cast is "assign this value to fred -- and by
the way treat it as a value of type char". What you're really telling
the compiler is, "Take the value of bill, coerce it to type char (I
don't char what type bill happens to be), and assign the result to
fred (I don't care what type fred happens to be) -- and don't
complain, because I promise you I know what I'm doing *wink wink*."

C performs implicit conversions in many contexts (too many for my
taste, but that's a separate issue). An explicit conversion via a
cast doesn't inhibit the implicit conversion; it's still performed
after the cast is performed. I'd often rather make my conversions
explicit rather than implicit, but C doesn't really give me that
choice.

[snip]

 

[beetle]

With the odd exception, don't cast malloc return.

Ehm sorry maybe I am to new to C, but how would you handle something
like structs then ?

What would be `the better' way of doing:
(struct tnode *)malloc(sizeof(struct tnode));

thnkx..

Beetle.

 

[pete]

Ehm sorry maybe I am to new to C, but how would you handle something
like structs then ?

What would be `the better' way of doing:
(struct tnode *)malloc(sizeof(struct tnode));

#include <stdlib.h>

struct tnode *pointer;

pointer = malloc(sizeof *pointer);

 

[E. Robert Tisdale]

#include <stdlib.h>

struct tnode *pointer;

pointer = malloc(sizeof *pointer);

typedef struct tnode tnode;

tnode* pointer = (tnode*)malloc(sizeof(tnode));

 

[Keith Thompson]

typedef struct tnode tnode;

tnode* pointer = (tnode*)malloc(sizeof(tnode));

Yes, that's a much worse way to do it.

 

[E. Robert Tisdale]

Yes, that's a much worse way to do it.

> cat tree.cc

#include <stdlib.h>

typedef struct tnode {
int data;
struct
tnode* pLeft;
struct
tnode* pRight;
} tnode;

int main(int argc, char* argv[]) {
tnode* pointer = malloc(sizeof(tnode));
free((void*)pointer);
return 0;
}

> g++ -Wall -ansi -pedantic -o tree tree.cc

tree.cc: In function `int main(int, char**)':
tree.cc:12: error: invalid conversion from `void*' to `tnode*'

My compiler doesn't like pete's way.
But it likes my way just fine.

 

[Peter Nilsson]

David REsnick writes:
[...]

With the odd exception, don't cast malloc return.

The "odd exception" you're referring to is writing code that needs to
compile as both C and C++, right?

Yes. I could have also said "unless your initials are P.J.P.",
as he seems to be the only one on the group who has expressed a
real need for that.

No, countless people expressed the same need before Plauger
entered the debate. [The possibility has been in the K&R2
errata for ages.]

 

[Michael Mair]

Correct standard way.

E.R.T.s way

Indeed.

We are in comp.lang.c; there is a high probabibility that
..cc indicates a C++ source.

#include <stdlib.h>

typedef struct tnode {
int data;
struct
tnode* pLeft;
struct
tnode* pRight;
} tnode;

int main(int argc, char* argv[]) {
tnode* pointer = malloc(sizeof(tnode));
free((void*)pointer);
return 0;
}

g++ -Wall -ansi -pedantic -o tree tree.cc

Call of a C++ compiler. Not surprisingly, it does not
perform the conversion from void * to tnode * as required
by the C standard.

tree.cc: In function `int main(int, char**)':
tree.cc:12: error: invalid conversion from `void*' to `tnode*'

My compiler doesn't like pete's way.
But it likes my way just fine.

Right. I just do not understand why you insist on spreading
misinformation which is completely beside the point.
The question was how this is better done in C. That C++ requires
the cast is a completely different matter as it is a different
language.


-Michael