How to release a struct that was passed by reference to c code and its member was malloc'ed there?

Question

I'm getting "malloc: * error for object 0xbfffe160: pointer being freed was not allocated" when trying to free memory (in objective-c code) of an object that was allocated inside c function. This C function creates and returns binary data packet that is used as NSData later. Here's my obj-c code part where I'm creating struct variable and passing it by reference to C function:

MyPacket packetRef;
allocAuthentificationCodePacket(&packetRef);

NSData *data = [NSData dataWithBytes:packetRef.bytes length:packetRef.packet->packetSize];
free(&packetRef); // getting error

Everything work fine, except I'm trying to release the memory because the data should be retained by NSData variable. The C functions performs calloc inside itself, so I should somehow to release that memory:

packetRef->bytes = calloc(1, sizeof(*packetRef));

Here's are my structs for storing binary data:

typedef struct {

  uint8_t packetType;
  uint16_t packetBody;

} MyStruct;

and another struct:

typedef union {

  const uint8_t *bytes; 
  MyStruct *packet;

} MyPacket;

How should I free the memory? The error I'm getting is not crash, it just a message in debug console when running unit tests.

UPDATE. Tried to release "bytes" struct member but getting the same error message:

free(&packetRef.bytes);

UPDATE2. Thanks, the suggested way did worked and malloc error message disappeared from console:

free(packetRef.bytes);

However, getting a warning in Xcode "Passing 'const uint8_t *' (aka 'const unsigned char *') to parameter of type 'void *' discards qualifiers". I'm using Apple LLVM 4.1 compiler. C function resides in separate file and only a header is included because Android guys will have to reuse it.

UPDATE3. Thanks to @simonc and @nos who have pointed out the struct member "bytes" has const. The warning has disappeared after removing const. The initial idea of using const was to protect "bytes" from modification.

Answer 1

This is always wrong. (Hint: It's almost always wrong to put & inside of free().)

MyPacket packetRef;
...
free(&packetRef); // getting error

It doesn't matter what MyPacket is -- it has automatic storage duration, i.e., the compiler automatically allocates storage and frees it when the function exits.

Do not free() something unless it came from malloc() (or calloc(), etc.)

Since packetRef.bytes was allocated with calloc(), you can free() that instead.

MyPacket packetRef;
allocAuthentificationCodePacket(&packetRef);
...
free(packetRef.bytes);

Update

If the function that you call, allocAuthentificationCodePacket, contains the code:

packetRef->bytes = calloc(1, sizeof(*packetRef));

And if the bytes field has type const uint8_t *, then something is wrong.

1. Perhaps your code is wrong, and you are supposed to call some function to free the packet rather than freeing it yourself.

2. Perhaps the type of the bytes field is wrong, and should be uint8_t * instead of const uint8_t *.

3. Perhaps allocAuthentificationCodePacket is wrong.

Who knows? It's not wrong enough to crash, but it is a problem.

Footnote

There are no references in C. &x is "address of x", not "reference to x".

Let's consider the following code:

char *x = malloc(10);
free(x);

When people talk about this code, they will say something like "x is allocated on the heap", but that's not technically correct, x is allocated on the stack and contains the address of 10 bytes on the heap. Likewise, the line free(x) does not actually free x, it frees the memory which x points to.

So when someone tells you, "don't forget to free x", you know they actually mean "don't forget to free the memory which the value contained in x points to". People are sloppy with terminology but computers aren't.

Answer 2

packetRef is a stack variable in your example with packetRef->bytes heap allocated. You should therefore call free(packetRef.bytes)

Since you allocate the memory inside a function - allocAuthentificationCodePacket - you may want to create another function to free the memory

void freePacket(MyPacket* packet)
{
    free(packet->bytes);
}