c free question

Question

Is this okay to do in c?

int *i;
// do stuff

i = NULL;

i = (int *) some_func();
// do stuff

if (i != NULL)
    free(i);

i = NULL;

// do stuff
i = (int *) some_func();
// do stuff
if (i != NULL)
    free(i);

i = NULL;

Answer 1

Your code looks OK - which specific bit are you asking about? But note that if the function returns an int *, there is no need for a cast, and if it doesn't you should probably not be assigning it to an int *.

Answer 2

That depends on what some_func() is doing. If it is allocating memory with malloc(), then you should free() it when you're done. If it is not, then you should not. Refer to the function's documentation to be sure.

Answer 3

If you mean is it OK to reuse the int pointer instead of declaring a new one with each use, sure it is OK but in general it can be confusing if all your // do stuff consists of a lot of code. Another programmer might get confused about where *i came from and why it means X here and Y there.

Answer 4

If some_func returns a pointer that points to a dynamically-allocated memory, yes.

Answer 5

It is OK as long as some_func() does what it is supposed to do. If it assigns an invalid (unallocated) address to i, it will cause your program to crash.

Answer 6

Well the only problem I see is the readability problem not tied just to C. You reused a variable name so many times in one block that it's really hard to find out what's it used for.

Answer 7

Depends on the contract of some_func().

If some_func() allocates some memory and designates it your responsibility to release it, then yes its fine to free() it. In fact, its a bug not to.

This is one of the bugbears of working in an unmanaged language, you have to keep track of what resources you own and be sure to release them.

Answer 8

If some_func() "owns" its data and only returns pointers to it for you to inspect, you should not free it. If it mallocs dat just for you then you are indeed responsible for doing the free

Answer 9

1) That depends on the contract you have with some_func(). If some_func expects you to call free() on its return value, your code is ok.

2) It's ok, though not terribly elegant, to reuse variables in C. It's generally better to use different variables for different purposes. From a performance and mem-usage perspective it completely the same thing.

Answer 10

I'd second Edgar's answer, but also note that the test for NULL here is unnecessary:

if (i != NULL)
    free(i);

because free(NULL) is allowed.

Answer 11

In your first step

int *i;
// do stuff

i = NULL;

If i pointed to something that allocated memory, won't that memory be lost forever (memory leak)?

For example:

int *i;
i = (int*) malloc(sizeof(int);
i = NULL;

would leave a chunk of memory the size of int left floating in the ether.

The same is true for your some_func() examples, but I think you're testing correctly to attempt to free up whatever some_func() left behind. Especially if you don't know how some_func() works.

If some_func() is from a library, there may be a free_some_func(i) function that would do the work for you.

Answer 12

i = (int *) some_func();

You didn't say what the return type of some_func() is, but the (int *) part is a bit suspicious. C is generally pretty lenient about these things, and usually compiles cleanly without the need for an explicit cast. If it doesn't, consider carefully if what you're doing is as well-defined and portable as you'd like it to be.

Answer 13

@Kevin Montrose: Hmmm.Yeah, requiring programmers to be competent is a real deal-breaker. Maybe we should all wear helmets while we sling code, just in case the ceiling falls in. And what is the "contract" of some_func? some_func either returns a value suitable to pass to free, or it doesn't. There is no "contract" there. But then, I am an old fart, and don't believe in obfuscation to win brownie points with management. These are simple concepts.

@caf: this is probably compiler/library-dependant. It's safer to check like he's doing. Just because your implementation of free checks for a NULL pointer doesn't mean they all do.