Returning dynamic memory inside a struct in C

Question

Hi I have the following struct

typedef struct mystruct_s {
    int* array;
} mystruct;

and I want to create a function that initializes it. I have two ways of doing it.

First one is:

mystruct new_mystruct()
{
    mystruct tmp;
    tmp.array = malloc(sizeof(int) * 5);

    return tmp;
}

Second one is:

mystruct* new_mystruct()
{
    mystruct* tmp = malloc(sizeof(mystruct));
    tmp->array = malloc(sizeof(int) * 5);

    return tmp;
}

I want to know which one is the better approach.

Thanks in advance!

Answer 1

A general rule of thumb is to never pass structs by value, mainly for performance reasons. The first version should be avoided for that reason.

Regarding the second version, then there's some more advanced issues. Multiple calls to malloc() have the following problems:

• Each call comes with execution overhead of its own.
• Each call may give a different segment, causing heap segmentation over time = poor usage of memory.
• Allocations in different segments means poor utilization of data cache on CPUs that support it.
• You need to make multiple calls to free() as well. Again extra execution overhead and the potential of memory leaks if you don't call it multiple times.

A better method is to use flexible array members:

typedef struct {
  size_t size
  int array[];
} mystruct;

size_t n = 123;
mystruct* ms = malloc( sizeof *ms + sizeof(int[n]) );
ms->size = n;
memcpy(ms->array, data, sizeof(int[n]));
...
free(ms);

This is faster and less complex all at once.

Answer 2

Both implementation are good, the first one uses heap memory by calling malloc() once which means you need to free() only once.

Whereas, in the second implementation, it uses heap memory by calling malloc() twice, so you need to call free() twice, which leads to additional calls to free() later in your code.

NOTE: Number of calls to malloc() should be equal to number of calls to free(). But in modern operating systems, sometimes freeing the heap allocated resources isn't needed, OS manage them accordingly.

Answer 3

I read the OP question wrong. I appreciate the comments pointing out my issue and making a better post. Here is an example of an array in a struct and a pointer in a struct. It also shows how the two variants differ in memory layout.

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>

struct one {
  int start;
  char pch[5];
  int end;
};

struct two {
  int start;
  char *pch;
  int end;
};

int main() {

  // Sizeof types
  // char 1 bytes/8-bits
  // int  4 bytes/32-bits
  // pointers 8 bytes/64-bits
  //                                              // Vars on stack in main frame.
  int main_start = 1;                             // &main_start = 0x7ffe74ad0b48
  struct one *pOne;                               // &pOne = 0x7ffe74ad0b50 +4 bytes due to int. +4 due to *boundary.
  struct two *pTwo;                               // &pTwo = 0x7ffe74ad0b58 +8 bytes due to pointer.
  int main_end = 2;                               // &main_end = 0x7ffe74ad0b4c +8 bytes due to pointer.
                                                  // * Pointer addresses on 8 byte boundaries.

  // allocate struct one and its components
  pOne = malloc(sizeof(struct one));
  pOne->start = 1;
  strcpy(pOne->pch, "test");
  pOne->end = 2;
  // values of struct to show they work
  printf("pOne->start = %d\n", pOne->start);
  printf("pOne->pch = %s\n", pOne->pch);
  printf("pOne->end = %d\n", pOne->end);
  // address of variables in main frame
  printf("&main_start = %p\n", &main_start);       // &main_start = 0x7ffe74ad0b48 see above.
  printf("&pOne = %p\n", &pOne);                   // &pOne = 0x7ffe74ad0b50
  printf("&pTwo = %p\n", &pTwo);                   // &pTwo = 0x7ffe74ad0b58
  printf("&main_end = %p\n", &main_end);           // &main_end = 0x7ffe74ad0b4c
  // address of variables in heap
  printf("pOne = %p\n", pOne);                     // pOne = 0x56126b025eb0   addr of struct in heap.
  printf("&(pOne->start) = %p\n", &(pOne->start)); // &(pOne->start) = 0x56126b025eb0. First element is same.
  printf("&(pOne->pch) = %p\n", &(pOne->pch));     // &(pOne->pch) = 0x56126b025eb4 +4 since int
  printf("&(pOne->end) = %p\n", &(pOne->end));     // &(pOne->end) = 0x56126b025ebc +5 since alloc. +3 boundary.


  printf("\n");

  // allocate struct two and its components
  pTwo = malloc(sizeof(struct two));
  pTwo->start = 1;
  pTwo->pch = malloc(sizeof(char)*5);
  strcpy(pTwo->pch, "TEST");
  pTwo->end = 2;
  // values of struct to show they work
  printf("pTwo->start = %d\n", pTwo->start);
  printf("pTwo->pch = %s\n", pTwo->pch);
  printf("pTwo->end = %d\n", pTwo->end);
  // address of variables in main frame
  printf("&main_start = %p\n", &main_start);
  printf("&pOne = %p\n", &pOne);
  printf("&pTwo = %p\n", &pTwo);
  printf("&main_end = %p\n", &main_end);
  // address of variables in heap
  printf("pTwo = %p\n", pTwo);                     // pTwo = 0x56126b0262e0  addr of struct in heap
  printf("&(pTwo->start) = %p\n", &(pTwo->start)); // &(pTwo->start) = 0x56126b0262e0 first element is same
  printf("&(pTwo->pch) = %p\n", &(pTwo->pch));     // &(pTwo->pch) = 0x56126b0262e8   +4 int +4 boundary
  printf("&(pTwo->end) = %p\n", &(pTwo->end));     // &(pTwo->end) = 0x56126b0262f0   +8 pointer

  // What I thought OP was doing since I did not read the code well enough.
  // My mistake.
  // struct foo {
  //   int *goo;
  // };
  // struct foo *pFoo;
  // pFoo = malloc(sizeof(foo)*5);
  // Failure since goo is just a pointer.  This is space for 5 foo
  // and the goo pointers are unallocated.
  printf("size of char %ld\n", sizeof(char));
  printf("size of int %ld\n", sizeof(int));
  printf("size of pointer %ld\n", sizeof(int*));

}