Free pointer from external function

Question

I wrote a program that uses the ADT stack.
The main one creates a new stack, providing 3 functions for the user:

Stack my_stack = sCreate (copy_int, free_int, print_int); 

when I call the peek function:

 printf ("Peek: |%d|\n\n", *(int*)sPeek(my_stack)); 

I have a memory leak.

peek function is as follows:

 Element sPeek (Stack stack){ if ((NULL == stack) || (0 >= stack->used_places)) return NULL; Element returnElement = stack->copy_function(stack->stack_array[stack->used_places-1]); if (NULL == returnElement){ free (returnElement); return NULL; } return returnElement; 

This is probably caused by the copy_function function, which is called there, which is copy_int, which was specified by the user:

 Element copy_int (Element element){ int *new_int = (int*) malloc(sizeof(int*)); *new_int = *(int*)element; if (NULL != new_int) return new_int; else return NULL; 

How to infer a pointer (malloc) from copy_int?

Answer 1

In the last code snippet, you use *new_int before checking the return value from malloc . This will cause a segmentation error if new_int is NULL . Also, if/else completely useless as written. These four lines can be replaced with return new_int; without any change in behavior under any circumstances. Finally, it does not discard the return value from malloc .

When fixing all these problems, the last piece of code looks like this:

 Element copy_int (Element element) { int *new_int = malloc(sizeof(int)); if ( new_int ) *new_int = *(int*)element; return new_int; } 

In the sPeek function, you have a similar useless if . If returnElement is NULL , nothing costs free . So the sPeek function should be

 Element sPeek (Stack stack) { if ( stack && stack->used_places > 0 ) return stack->copy_function(stack->stack_array[stack->used_places-1]); else return NULL; } 

Finally, to your question, the memory returned by copy_int will be skipped unless you save a copy of this pointer and free when you are done with it. Also, you are requesting another segmentation error if you pass a NULL pointer to printf . Therefore, the printf line should be replaced with this code (assuming that Element really void * )

 int *value = sPeek(my_stack); if (value) printf ("Peek: |%d|\n\n", *value); free(value); 

Answer 2

 Element e = sPeek(my_stack); if (e) { printf ("Peek: |%d|\n\n", *(int*)e); } free(e); 

It seems a bit obvious, so I'm not sure what you meant.

Answer 3

How to infer a pointer (malloc) from copy_int?

Just call free() on it if you don't need it anymore.

---

Also here

 int * new_int = (int*) malloc(sizeof(int*)); *new_int = *(int*)element; if (NULL != new_int) return new_int; else return NULL; 

a test for NULL must be completed before the element pointer is deleted:

 int *new_int = malloc(sizeof(int*)); if (NULL != new_int) { *new_int = *(int*)element; return new_int; } else return NULL; 

Note. Executing the result malloc/calloc/realloc not required in C, and it is not recommended in any way.

---

Also ^ 2 call free() here:

 if (NULL == returnElement){ free (returnElement); return NULL; } 

less used since there is nothing free() since returnElement has no value b that is NULL . You want to delete it.

Answer 4

Any function that returns a resource that is not automatically released after use should have documentation on how to free the resource. In the case of malloc() it is documented as free() , for fopen() it is fclose() , etc.

When you create a function yourself, you can, for example, refer to free() if you return a pointer, which you, in turn, received from malloc() . If you have a more complicated setup, you may have to create your own function.

Looking at your function, you allocate memory using malloc() , then assign it to memory (or explode if the allocation fails, which you check too late), and then return exactly the pointer obtained from malloc() . Therefore, you are returning a resource that can (and should!) Be freed using free() .

By the way: think about not copying things unnecessarily. Your call to copy_int() seems superfluous to me, just return a pointer to const int, referring to the existing element, and you should be fine.