C free(): invalid pointer allocated in other function

Question

I'm new in StackOverflow. I'm learning C pointer now.

This is my code:

#include <stdio.h>
#include <stdlib.h>

int alloc(int* p){
    p = (int*) malloc (sizeof(int));
    if(!p){
        puts("fail\n");
        return 0;
    }
    *p = 4;
    printf("%d\n",*p);
    return 1;
}

int main(){

    int* pointer;

    if(!alloc(pointer)){
        return -1;
    }else{

        printf("%d\n",*pointer);
    }

    free(pointer);

    return 0;
}

I compile with: gcc -o main main.c

error: free(): invalid pointer: 0xb77ac000 ***

what's wrong with my code?

Answer 1

Arguments in C are always passed by value. So, when you call alloc(pointer), you just pass in whatever garbage value pointer contains. Inside the function, the assignment p = (int*)... only modifies the local variable/argument p. Instead, you need to pass the address of pointer into alloc, like so:

int alloc(int **p) {
    *p = malloc(sizeof(int)); // side note - notice the lack of a cast
    ...
    **p = 4; // <---- notice the double indirection here
    printf("%d\n", **p); // <---- same here
    return 1;
}

In main, you would call alloc like this:

if (!(alloc(&pointer))) {
    ....

Then, your code will work.

Answer 2

Everything in C is pass-by-value. This means that functions always operate on their own local copy of what you pass in to the function. Usually pointers are a good way to mimic a pass-by-reference scheme because a pointer and a copy of that pointer both contain the same memory address. In other words, a pointer and its copy both point to the same space.

In your code the issue is that the function alloc gets its own local copy of the pointer you're passing in. So when you do p = (int*) malloc (sizeof(int)); you're changing the value of p to be a new memory address, but the value of pointer in main remains unchanged.

You can get around this by passing a pointer-to-a-pointer, or by returning the new value of p.

Answer 3

You have two major problems in your code.

First, the alloc function creates a pointer via malloc, but never frees it, nor does it return the pointer to the calling function. This guarantees the memory the pointer addresses can never be freed up via the free command, and you now have memory leaks.

Second, the variable, int* pointer in main, is not being modified as you would think. In C, function arguments are "passed by value". You have two ways to address this problem:

1. Pass a pointer to the variable you want to modify (in your case, a pointer to a pointer to an int)
2. Have the function return the pointer to the function that called it.

Here are two implementations of my recommendations:

Approach 1

---

#include <stdio.h>
#include <stdlib.h>

int alloc(int** p);

int alloc(int** p) {
   if (!p) {
      printf("Invalid argument\n");
      return (-1);
   }
   if ((*p = (int*)malloc(sizeof(int))) == NULL) {
      printf("Memory allocation error\n");
      return (-1);
   }
   **p = 123;
   printf("p:%p - *p:%p - **p:%d\n", p, *p, **p);
   return 0;
}

int main(){
   int* pointer;

   if(alloc(&pointer) != 0){
      printf("Error calling function\n");
   }else{
      printf("&pointer:%p- pointer:%p- *pointer:%d\n", &pointer, pointer, *pointer);
   }

   free(pointer);

   return 0;
}

Sample Run for Approach 1

---

p:0xbfbea07c - *p:0x8656008 - **p:123
&pointer:0xbfbea07cointer - pointer:0x8656008ointer - *pointer:123

Approach 2

---

#include <stdio.h>
#include <stdlib.h>

int* alloc(void) {
   int* p;
   if ((p = (int*)malloc(sizeof(int))) == NULL) {
      printf("Memory allocation error\n");
      return (NULL);
   }
   *p = 123;
   printf("p:%p - *p:%d\n", p, *p);
   return p;
}

int main(){
   int* pointer = alloc();

   if(pointer == NULL) {
      printf("Error calling function\n");
   }else{
      printf("&pointer:%p- pointer:%p- *pointer:%d\n", &pointer, pointer, *pointer);
   }

   free(pointer);
   pointer = NULL;

   return 0;
}

Sample Run for Approach 2

---

p:0x858e008 - *p:123
&pointer:0xbf9bb1ac- pointer:0x858e008- *pointer:123

Answer 4

You are passing the pointer by value into your alloc function. Although that function takes a pointer to an int, that pointer itself cannot be modified by the function. If you make alloc accept **p, set *p = ..., and pass in &pointer from main, it should work.

#include <stdio.h>
#include <stdlib.h>
int alloc(int** p){
  *p = (int*) malloc (sizeof(int)); 
  if(!*p){
    puts("fail\n"); 
    return 0; 
  } 
  **p = 4; 
  printf("%d\n",**p);
  return 1; 
} 
int main() { 
  int* pointer; 
  if(!alloc(&pointer)){ 
    return -1;
  } else { 
    printf("%d\n",*pointer);
  } 
  free(pointer);
  return 0;
}

Answer 5

If you want a function to write to a non-array parameter of type T, you must pass a pointer to that parameter.

void func( T *ptr )
{
  *ptr = new_value;
}

void foo ( void )
{
  T var;
  func( &var ); // writes new value to var
}

If T is a pointer type Q *, it would look like

void func( Q **ptr )
{
  *ptr = new_pointer_value;
}

void foo ( void )
{
  Q *var;
  func( &var ); // writes new pointer value to var
}

If Q is a pointer type R *, you would get

void func( R ***ptr )
{
  *ptr = new_pointer_to_pointer_value;
}

void foo ( void )
{
  R **var;
  func( &var ); // writes new pointer to pointer value to var
}

The pattern is the same in all three cases; you're passing the address of the variable var, so the formal parameter ptr has to have one more level of indirection than the actual parameter var.

One sylistic nit: instead of writing

p = (int *) malloc( sizeof (int) );

use

p = malloc( sizeof *p );

instead.

In C (as of the 1989 standard), you don't need to cast the result of malloc; void pointers can be assigned to other pointer types and vice versa without needing a cast (this is not true in C++, but if you're writing C++, you should be using the new operator instead of malloc anyway). Also, under the 1989 version of the language, using the cast would mask a bug if you forgot to include stdlib.h or otherwise didn't have a declaration for malloc in scope. That hasn't been a problem since the 1999 version, though, so now it's more a matter of readability than anything else.

The type of the expression *p is int, so the result of sizeof *p is the same as the result of sizeof (int). This way, if you ever change the type of p, you don't have to modify the malloc call.

To allocate an array of values, you'd use something like

T *p = malloc( sizeof *p * NUM_ELEMENTS );

or, if you want everything to be zeroed out initially, use

T *p = calloc( sizeof *p, NUM_ELEMENTS );