views:

536

answers:

6

I need help with malloc() inside another function.

I'm passing a pointer and size to the function from my main() and I would like to allocate memory for that pointer dynamically using malloc() from inside that called function, but what I see is that.... the memory, which is getting allocated, is for the pointer declared within my called function and not for the pointer which is inside the main().

How should I pass a pointer to a function and allocate memory for the passed pointer from inside the called function?


I have written the following code and I get the output as shown below.

SOURCE:

main()
{
   unsigned char *input_image;
   unsigned int bmp_image_size = 262144;

   if(alloc_pixels(input_image, bmp_image_size)==NULL)
     printf("\nPoint2: Memory allocated: %d bytes",_msize(input_image));
   else
     printf("\nPoint3: Memory not allocated");     

}

signed char alloc_pixels(unsigned char *ptr, unsigned int size)
{
    signed char status = NO_ERROR;
    ptr = NULL;

    ptr = (unsigned char*)malloc(size);

    if(ptr== NULL)
    {
        status = ERROR;
        free(ptr);
        printf("\nERROR: Memory allocation did not complete successfully!");
    }

    printf("\nPoint1: Memory allocated: %d bytes",_msize(ptr));

    return status;
}

PROGRAM OUTPUT:

Point1: Memory allocated ptr: 262144 bytes
Point2: Memory allocated input_image: 0 bytes
+9  A: 

You need to pass a pointer to a pointer as the parameter to your function.

main()
{
   unsigned char *input_image;
   unsigned int bmp_image_size = 262144;

   if(alloc_pixels(&input_image, bmp_image_size)==NULL)
     printf("\nPoint2: Memory allocated: %d bytes",_msize(input_image));
   else
     printf("\nPoint3: Memory not allocated");     

}

signed char alloc_pixels(unsigned char **ptr, unsigned int size) 
{ 
    signed char status = NO_ERROR; 
    *ptr = NULL; 

    *ptr = (unsigned char*)malloc(size); 

    if(*ptr== NULL) 
    {
        status = ERROR; 
        free(*ptr);      /* this line is completely redundant */
        printf("\nERROR: Memory allocation did not complete successfully!"); 
    } 

    printf("\nPoint1: Memory allocated: %d bytes",_msize(*ptr)); 

    return status; 
} 
Mark Ransom
<S>Why are you calling free in a conditional code block that is guaranteed to have a NULL pointer!?!?</S> The `free(*ptr)` will when called from `main()` try to free `input_image` which was ummm, the term evades me... not dynamically allocated.
James Morris
@Mark and @James: I did what was suggested by Mark and Matti, but this time both my _mize(input_image) in my main() and _msize(**ptr) in my alloc_pixels(...) function are returning the size as 0.Whereas if it is _msize(*ptr) (single *) returns 262144. ?
vikramtheone
@James Morris, I just copied the code that was posted in the question and made the minimal number of changes. I didn't want to get caught up in a distraction to the main point.
Mark Ransom
@vikramtheone, sorry I was a bit rushed and didn't make this answer as complete as it should have been. I've edited it to be more complete. I hope you can see how it is different than your original code and why it must be this way.
Mark Ransom
+4  A: 

If you want your function to modify the pointer itself, you'll need to pass it as a pointer to a pointer. Here's a simplified example:

void allocate_memory(char **ptr, size_t size) {
    void *memory = malloc(size);
    if (memory == NULL) {
        // ...error handling (btw, there's no need to call free() on a null pointer. It doesn't do anything.)
    }

    *ptr = (char *)memory;
}

int main() {
   char *data;
   allocate_memory(&data, 16);
}
Matti Virkkunen
It's safe to call `free()` on a null pointer, what is that comment about?
Carl Norum
@Carl Norum: It's safe, but pointless. IMO, code that doesn't do anything only leads to confusion for people who end up reading it later and should be avoided.
Matti Virkkunen
@Matti Virkkunen: Telling people to not call free on a NULL pointer is pointless *and* misinformation - you're causing people to become confused when they see code that goes against your advice.
James Morris
@James Morris: Fine, fine... like the wording better now?
Matti Virkkunen
@Carl: I've encountered (not very nice) C libraries that crashed if asked to `free(NULL);` so it's good to avoid anyway. (No, I don't remember which. It was quite a while ago.)
Donal Fellows
@Donal Fellows: Those C libraries are non-conforming, then. The standard requires `free(NULL)` to do nothing.
jamesdlin
@Donal Fellows: Sounds like a horribly broken standard library to me... must have really been a while ago
Matti Virkkunen
@Matti Virkkunen: yes :-)
James Morris
@All here :)I did what was suggested by Mark and Matti, but this time both my _mize(input_image) in my main() and _msize(**ptr) in my alloc_pixels(...) function are returning the size as 0.Whereas if it is _msize(*ptr) (single *) returns 262144. ?
vikramtheone
@vikramtheone: I don't see what the problem is, the return values seem to be correct.
Matti Virkkunen
@Matti: How can I find out the size of memory just allocated using the input_image pointer? Why does _msize(input_image) return a 0 and not the size of memory?
vikramtheone
@vikramtheone: Because inside alloc_pixels, ptr is a pointer to the stack of the calling function and not the heap. You need to dereference ptr by doing *ptr to get to the heap address.
Matti Virkkunen
@Matti: It was back in the early '90s, and was considered an old std-lib then. Of course, back then we also had all the ridiculous nastiness of `far` pointers, memory models and other things like that, which I'm convinced still leave their mark in standards today (such as in the lack of a guarantee that pointers to data and pointers to functions are the same size).
Donal Fellows
In short, life is *much* better now. :-)
Donal Fellows
+2  A: 

This does not make sense :

if(alloc_pixels(input_image, bmp_image_size)==NULL) 

alloc_pixels returns a signed char (ERROR or NO_ERROR) and you compare it to NULL (which is supposed to be used for pointers).

If you want input_image to be changed, you need to pass a pointer to it to alloc_pixels. alloc_pixels signature would be the following:

signed char alloc_pixels(unsigned char **ptr, unsigned int size)

You would call it like this:

alloc_pixels(&input_image, bmp_image_size);

And the memory allocation

*ptr = malloc(size);
Bertrand Marron
+10  A: 

How should I pass a pointer to a function and allocate memory for the passed pointer from inside the called function?

Ask yourself this: if you had to write a function that had to return an int, how would you do it?

You'd either return it directly:

int foo(void)
{
    return 42;
}

or return it through an output parameter by adding a level of indirection (i.e., using an int* instead of int):

void foo(int* out)
{
    assert(out != NULL);
    *out = 42;
}

So when you're returning a pointer type, it's the same thing: you either return the pointer type directly:

T* foo(void)
{
    T* p = malloc(...);
    return p;
}

or you add one level of indirection:

void foo(T** out)
{
    assert(out != NULL);
    *out = malloc(...);
}
jamesdlin
Thanks for giving the explanation I was too rushed to provide.
Mark Ransom
+1, I love everything but the assertion, however its fine for such a simple demonstration.
Tim Post
I like the assertion; it is a part of the contract for the function which the caller should get systematically correct. Of course, even more subtle code might make a NULL `out` allowable, having it correspond to an optional out-parameter. But that's not what is needed for `alloc_pixels`; the question does not require such sophistication.
Donal Fellows
+2  A: 
tommieb75
@tommieb75: I understand what wrong I was doing. There is however one issue still not solved. When I make these changes and use _msize(input_image); in my main(), the _msize(...) returns a 0. At the same time for _msize(*ptr); in the other function, I get the size as 262144. What's going wrong here? I have no clue.
vikramtheone
@vikramtheone: can you show the function prototype for _msize(...) please? Amend your question to highlight that...
tommieb75
@tommieb75: The _msize(...) is part of malloc.h check this out there is also an example there.http://msdn.microsoft.com/en-us/library/Aa298504
vikramtheone
@tommieb75: Never mind, it works fine now :) It was a late-late night work and my mind had all become fuzzy and I forgot to change the main(). I was not sending the address of input_image when I calling the alloc_memory(...) in main().
vikramtheone
+1  A: 

Hi,

In your initial code , when you were passing input_image to the function alloc_pixels, compiler was creating a copy of it (i.e. ptr) and storing the value on the stack. You assign the value returned by malloc to ptr. This value is lost once the function returns to main and the stack unwinds. So, the memory is still allocated on heap but the memory location was never stored in (or assigned to )input_image, hence the issue.

You can change the signature of the function alloc_pixels which would be simpler to understand, and you won't require the additional 'status' variable as well.

unsigned char *alloc_pixels(unsigned int size)
{
    unsigned char *ptr = NULL;
    ptr = (unsigned char *)malloc(size);
    if (ptr != NULL)
       printf("\nPoint1: Memory allocated: %d bytes",_msize(ptr));
    return ptr;
}

You can call the above function in main :

int main()
{
   unsigned char *input_image;
   unsigned int bmp_image_size = 262144;

   if((input_image = alloc_pixels(bmp_image_size))==NULL)
       printf("\nPoint3: Memory not allocated");    
   else
     printf("\nPoint2: Memory allocated: %d bytes",_msize(input_image)); 
   return 0;

}
juventus