Reputation: 419
I am new in C, trying to figure out about memory allocation in C that I kinda confused
#include <stdio.h>
#include <stdlib.h>
typedef struct
{
int a;
} struct1_t;
int main()
{
funct1(); //init pointer
return 1;
}
int funct2(struct1_t *ptr2struct)
{
printf("print a is %d\n",ptr2struct->a);
//free(ptr2struct);
printf("value of ptr in funct2 is %p\n", ptr2struct);
return 1; //success
}
int funct1(){
struct1_t *ptr2struct = NULL;
ptr2struct = malloc(sizeof(*ptr2struct));
ptr2struct->a = 5;
printf("value of ptr before used is %p", ptr2struct);
if (funct2(ptr2struct) == 0) {
goto error;
}
free(ptr2struct);
printf("value of ptr in funct1 after freed is is %p\n", ptr2struct);
return 1;
error:
if(ptr2struct) free(ptr2struct);
return 0;
}
I have funct 1 that calls funct 2, and after using the allocated pointer in funct1, I try to free the pointer. And I create a case where if the return value in funct2 is not 1, then try again to free the pointer.
My question is below
which practice is better, if I should free the memory in funct2 (after I pass it) or in funct1 (after I finish getting the return value of funct1) The second thing is whether this is correct to make a goto error, and error:
if(ptr2struct) free(ptr2struct);
My third question is , how do I check if the allocated value is already freed or not? because after getting the return value, I free the pointer, but if I print it, it shows the same location with the allocated one (so not a null pointer).
Upvotes: 19
Views: 106137
Reputation: 1
I know this is answered but, I wanted to give my input. As far as I understand, when you call a function with parameters such as here (the pointer), the parameters are pushed to the stack(FILO).
Therefore the pointer passed to the function will be automagically popped off the stack but not freeing the pointer in funct1(). Therefore you would need to free the pointer in funct1() Correct me if I am wrong.
Upvotes: 0
Reputation: 34527
Calling free() on a pointer doesn't change it, only marks memory as free. Your pointer will still point to the same location which will contain the same value, but that value can now get overwritten at any time, so you should never use a pointer after it is freed. To ensure that, it is a good idea to always set the pointer to NULL after free'ing it.
Upvotes: 20
Reputation: 22382
What i am about to share are my own development practices in C. They are by NO mean the ONLY way to organize yourself. I am just outlining a way not the way.
Okay, so, in many ways "C" is a loose language, so a lot of discipline and strictness comes from oneself as a developer. I've been developing in "C" for more than 20 years professionally, I've only very rarely have I had to fix any production-grade software that I have developed. While quite a bit of the success may be attributed to experience, a fair chunk of it is rooted in consistent practice.
I follow a set of development practices, which are quite extensive, and deal with everything as trivial as tabs to naming conventions and what not. I will limit my self to what I do about dealing with structures in general and there memory management in particular.
If I have a structure that's used throughout the software, I write create/destroy; init/done type functions for it:
struct foo * init_foo();
void done_foo(struct foo *);
and allocate and de-allocate the structure in these functions.
If I manipulate a structure elements directly all over the program then don't typedef it. I take the pain of using the struct keyword in each declaration so that I know it's a structure. This is sufficient where the pain threshold is NOT so much that I would get annoyed by it. :-)
If I find that the structure is acting VERY much like an object then I choose to manipulate the structure elements STRICTLY through an opaque API; then I define its interface through set/get type functions for each element, I create a 'forward declaration' in the header file used by every other part of the program, create a an opaque typedef to the pointer of the structure, and only declare the actual structure in the structure API implementation file.
foo.h:
struct foo;
typedef struct foo foo_t;
void set_e1(foo_t f, int e1);
int get_ei(foo_t f);
int set_buf(foo_t f, const char *buf);
char * get_buf_byref(foo_t f)
char * get_buf_byval(foo_t f, char *dest, size_t *dlen);
foo.c:
#include <foo.h>
struct foo {
int e1;
char *buf;
...
};
void set_e1(foo_t f, int e1) {
f->e1 = e1;
}
int get_ei(foo_t f) { return f->e1; }
void set_buf(foo_t f, const char *buf) {
if ( f->buf ) free ( f->buf );
f->buf = strdup(buf);
}
char *get_buf_byref(foo_t f) { return f->buf; }
char *get_buf_byval(foo_t f, char **dest, size_t *dlen) {
*dlen = snprintf(*dest, (*dlen) - 1, "%s", f->buf); /* copy at most dlen-1 bytes */
return *dest;
}
You will see a strong similarity between the approach i've outlined above and object oriented programming. It is meant to be that ...
If you keep your interfaces clean like this, whether or not you have to set instance variables to NULL all over the place won't matter. The code will, hopefully, yield itself to a tighter structure where silly mistakes are less likely.
Hope this helps.
Upvotes: 2
Reputation: 69944
1) Should I free it in the calling function or in the called function?
I try to do the free-ing in the same function that does the malloc-ing. This keeps the memory-management concerns in one place and also gives better separation of concerns, since the called function in this case can also work with pointers that have not been malloc-ed or use the same pointer twice (if you want to do that).
2) Is it correct to do a "goto error"?
Yes! By jumping to a single place at the end of the function you avoid having to duplicate the resource-releasing code. This is a common pattern and isn't that bad since the "goto" is just serving as a kind of "return" statement and isn't doing any of its really tricky and evil stuff it is more known for.
//in the middle of the function, whenever you would have a return statement
// instead do
return_value = something;
goto DONE;
//...
DONE:
//resorce management code all in one spot
free(stuff);
return return_value;
C++, on the other hand, has a neat way to do this kind of resource management. Since destructors are deterministically called right before a function exits they can be used to neatly package this king of resource management. They call this technique RAII
Another way other languages have to deal with this is finally blocks.
3) Can I see if a pointer has already been freed?
Sadly, you can't. What some people do is setting the pointer variable value to NULL after freeing it. It doesn't hurt (since its old value shouldn't be used after being freed anyway) and it has the nice property that freeing a null pointer is specified to be a no-op.
However, doing so is not foolproof. Be careful about having other variables aliasing the same pointer since they will still contain the old value, that is now a dangerous dangling pointer.
Upvotes: 16
Reputation: 2883
My question is below
which practice is better, if I should free the memory in funct2 (after I pass it) or in funct1 (after I finish getting the return value of funct1)
This is an "ownership" question. Who owns the allocated memory. Typically, this has to be decided based on the design of your program. For example, the only purpose of func1() could be to only allocate memory. That is, in your implementation, func1() is the function for memory allocation and then the "calling" function uses the memory. In that case, the ownership to free the memory is with the caller of func1 and NOT with func1().
The second thing is whether this is correct to make a goto error, and error: The use of "goto" is generally frowned about. It causes mess in the code that could just be easily avoided. However, I say "generally". There are cases where goto can be quiet handy and useful. For example, in big systems, configuration of the system is a big step. Now, imagine you call a single Config() function for the system which allocates memory for its different data structures at different points in the function like
config()
{
...some config code...
if ( a specific feature is enabled)
{
f1 = allocateMemory();
level = 1;
}
....some more code....
if ( another feature is enabled)
{
f2 = allocateMemory();
level = 2;
}
....some more codee....
if ( another feature is enabled)
{
f3 = allocateMemor();
level =3;
}
/*some error happens */
goto level_3;
level_3:
free(f3);
level_2:
free(f2);
level_1:
free(f1);
}
In this case, you can use goto and elegantly free only that much memory that was allocated till the point the configuration failed.
However, suffice to say in your example goto is easily avoidable and should be avoided.
My third question is , how do I check if the allocated value is already freed or not? because after getting the return value, I free the pointer, but if I print it, it shows the same location with the allocated one (so not a null pointer).
Easy. Set the freed memory as NULL. The other advantage, apart from the one mentioned by MK, is that passing NULL pointer to free will cause a NOP i.e. no operation is performed. This will also help you avoid any double delete problems.
Upvotes: 1