CODEWITHSUNDEEP
cmemorystructmallocfree
Filipe Nóbrega
Filipe Nóbrega

Reputation: 667

Free memory from data structure

I have a data structure that contains a size and some data. I want to free the memory when I allocate it.

My struct:

struct test_t {
    int size; 
    void *data;   
};

In order to create my structure I alloc test->data and then alloc struct test_t. Like shown:

struct teest_t *data_create(int size, void *data){
  if (size <= 0 || data == NULL) return NULL;

  struct test_t *new_data = malloc( sizeof(struct test_t));

  new_data->size = size;

  new_data->data = malloc(sizeof(*data));
  new_data->data = data;

  return new_data;
}

Now when I want to free my memory it causes a Segmentation fault (core dumped). This is my function:

void data_destroy(struct test_t *data){

  if (data->data == NULL) free(data->data);
  free(data);
}

Upvotes: 0

Views: 392

Answers (2)

KamilCuk
KamilCuk

Reputation: 140960

new_data->data = malloc(sizeof(*data));
new_data->data = data;

You first allocate memory, then loose the pointer (memory leak) and overwrite the pointer value with data pointer value. The second line copies pointer value, not bytes behind pointers. For that use memcpy.

You probably want:

new_data->data = malloc(size);
memcpy(new_data->data, data, size);

This will allocate new memory block size bytes big and copy the size bytes of data pointed to by data pointer.

Example:

#include <stdlib.h>
#include <stddef.h>
#include <string.h>

struct test_t {
    size_t size; 
    void *data;   
};

struct test_t *data_create(size_t size, void *data){
  if (size <= 0 || data == NULL) return NULL;

  struct test_t *new_data = malloc(sizeof(struct test_t));
  if (new_data == NULL) {
     goto NEW_DATA_MALLOC_ERR;
  }

  new_data->size = size;
  new_data->data = malloc(size);
  if (new_data->data == NULL) {
      goto NEW_DATA_DATA_MALLOC_ERR;
  }
  memcpy(new_data->data, data, size);

  return new_data;

NEW_DATA_DATA_MALLOC_ERR:
  free(new_data);
NEW_DATA_MALLOC_ERR:
  return NULL;
}

void data_destroy(struct test_t *data) {

  if (data->data == NULL) {
      free(data->data);
  }
  free(data);
}

int main() {
    int object = 1;
    struct test_s *A = data_create(sizeof(object), &object);
    data_destroy(A);
}

Upvotes: 1

0___________
0___________

Reputation: 67476

It is wrong of course. Is you save the reference to the data the second malloc is not needed at all.

If you want to save the data itself you need to allocate the buffer of the size of the data and then copy the data to the allocated buff.

The version you have now only leaks the memory. You do not have to allocate memory for the pointer, only for the actually data

Upvotes: 4

Related Questions