Dynamic memory allocation for structs in c

Question

I am trying to initialise a struct by allocating memory to it and it's pointer members using malloc:

typedef struct {
    char *name;
    prob_t *prob;
} name_t;

I understand that I need to allocate memory to the pointers separately once the struct is initialised:

name_t
*init_name_dict() {
    name_t *name_dict;
    name_dict = (name_t*)malloc(MAX_LINES*sizeof(*name_dict));
    name_dict->name = (char*)malloc(MAX_LEN*sizeof(*name_dict->name));
    name_dict->prob = (prob_t*)malloc(MAX_PROB*sizeof(*name_dict->prob));
    return name_dict;
}

But when I do so, it allocates memory to the struct, but not to either of its member pointers (they just point to junk).

What am I doing wrong? Thanks

The `malloc` method doesn't "clean" the memory it returns. Use `calloc` if you want zeroed memory, or `memset` it. See https://stackoverflow.com/questions/1538420/difference-between-malloc-and-calloc — xanatos, May 23 '18 at 13:11
Your code looks fine to me, except that you do not check `malloc()`'s return value and that it is unnecessary to cast the return value of `malloc()` in C. And note well the distinction implicit in @xanatos's comment: a valid pointer to space with junk in it is not at all the same thing as an invalid pointer. When `malloc()` fails, it returns a null pointer. — John Bollinger, May 23 '18 at 13:27

xanatos · Answer 1 · 2018-05-23T14:51:01.640

As explained here, malloc doesn't "clean" the memory, that then can be full of garbage (because for example the same memory was returned by another call to malloc(), used and then free()). The three classical solutions are:

Live with it. Set manually all the members of the struct (if you are using the malloc to allocate a struct) before using the struct (or in general set the all the obtained memory to the value you want)
Use memset to zero all the memory before using it
Use calloc instead of malloc (note that it has a slightly different signature). calloc is similar to malloc + memset. As an example:

name_t *init_name_dict() {
    name_t *name_dict;
    name_dict = calloc(MAX_LINES, sizeof(*name_dict));
    name_dict->name = calloc(MAX_LEN, sizeof(*name_dict->name));
    name_dict->prob = calloc(MAX_PROB, sizeof(*name_dict->prob));
    return name_dict;
}

As a sidenote, in C you don't need/shouldn't cast a pointer returned by malloc/calloc (but if in truth you are using a C++ compiler then you have to cast it...).

Petr Skocik · Answer 2 · 2018-05-23T14:57:14.173

If you want cleared memory (as opposed to memory with junk in it), you need calloc instead of malloc, but that's trivial.

You're bigger problems are:

1) no error checking
2) possibly needless malloc calls
3) you're allocating MAX_LINES of theses name_t structure but initializing 
  only one of them

If the .name and .prob fields won't be reallocated, you should change your name_t definition to

typedef struct { char name[MAX_LEN]; prob_t prob[MAX_PROB]; } name_t;

and allocate all MAX_LINES name_t's in one go: calloc(MAX_LINES, sizeof(name_t)).

If you need the original name_t structure, then I'd have an initializer for one:

int init_name_dict (name_t  *this)
{
    if(0==(this->name=calloc(MAX_LEN, sizeof *this->name))) return -1;
    if(0==(this->prob=calloc(MAX_PROB, sizeof *this->prob))){ free(this->name); return -1; }
    return 0;
}

a destructor for it

void destroy_name_dict(name_t *this) { free(this->name); free(this->prob); }

and then an initializing allocator for the whole array:

name_t* new_name_dicts(void)
{
    name_t *r = malloc(MAX_LINES*sizeof *r);
    if(!r) return r;
    int i;
    for(i=0; i<MAX_LINES; i++)
        if(0>init_name_dict(&r[i])) goto fail;
    return r;
    fail:
        for(--i; i>=0; --i)
            destructor_name_dict(&r[i]);
    return NULL;
}

(Basically what would amount to a C++ vector constructor that picks up the constructor for the cell type.)

s.paszko · Answer 3 · 2018-05-23T15:08:35.197

Struct

typedef struct {
    char *name;
    prob_t *prob;
} name_t;

has two pointers as members. So on 32 bit OS, sizeof(name_t) is 8 bytes. Dynamic creation of instance of name_t struct

name_t *name_dict = (name_t*)malloc(sizeof(name_dict));

allocates only 8 bytes to store two pointers. As xanatos said allocated memory is garbage and pointers will point random locations. You can use calloc() when allocation name_dict or manually nullify them name_dict->name = NULL;name_dict->prob = NULL;. You can also don't bother yourself by content of pointers and in next code line allocate memory to members

name_dict->name = (char*)malloc(MAX_LEN*sizeof(char));
name_dict->prob = (prob_t*)malloc(sizeof(prob_t));

You can also check if memory was allocated good and both pointers don't ppoint to NULL.

To sum up, properly written init_name_dict() method

name_t * init_name_dict() 
{
  name_t *name_dict = (name_t*)malloc(sizeof(name_t));
  if  (name_dict != NULL)
  {
        name_dict->name = (char*)malloc(MAX_LEN*sizeof(char)));
        name_dict->prob = (prob_t*)malloc(sizeof(prob_t));
  }
  return name_dict;
 }

Errors in your code was

MAX_LINES here(assume that you want to create only one structure here)

name_dict = (name_t*)malloc(MAX_LINES*sizeof(*name_dict));
MAX_PROB here (assume that you want to create only one structure here)

name_dict->prob = (prob_t*)malloc(MAX_PROB*sizeof(*name_dict->prob));

You seem to have misunderstood the OP's main problem. Although his code would be more robust if it checked the results of his `malloc()` calls, it is basically ok. His is a problem of wrong expectations about the contents of the memory to which his (properly allocated) pointers point. — John Bollinger, May 23 '18 at 13:37
This `name_t *name_dict = (name_t*)malloc(sizeof(name_dict));` is wrong, as it allocates just the size of a pointer. The OP does it right. — alk, May 23 '18 at 14:30
You missed to correct this "*So on 32 bit OS, `sizeof(name_t)` is 8 bytes.*" — alk, May 23 '18 at 15:36
Also why do you mention this: "*Dynamic creation of instance of `name_t` struct `name_t *name_dict = (name_t*)malloc(sizeof(name_dict));` allocates only 8 bytes to store two pointers.*"? The OP is not doing it this way. — alk, May 23 '18 at 15:38

Dynamic memory allocation for structs in c

3 Answers3