Allocate space for struct pointer member through constructor

Question

I want to use structs like objects in C.

Suppose I have the following:

typedef struct {
    /* ... */
    size_t *pages_len;
} book;

And I use the following method to construct it:

int book_init(/* some args... */, book * b) {
    /* do some validation */
    /* compute the number of pages n_pages */
    b->pages_len = (size_t*) calloc(n_pages, sizeof(size_t));
    /* compute pages_len based on some args */
    return 0;
}

Then I construct an object like this:

book *my_book = (book*)malloc(sizeof(my_book));
if (book_init(/* some args */, my_book) == 0) {
    /* do something */
}

And I destroy my object like: book_destroy(book *b) where I free(b->pages_len).

Is this correct or am I missing something? I can't show the original code but I am having trouble:

1. Accessing b->pages_len after the init method.
2. Destroying the object. I am having memory corruption.

As requested, a minimal reproducible example:

/* book.h */
#ifndef BOOK_HEADER_
#define BOOK_HEADER_
#include <ctype.h>

typedef struct
{
  size_t pages_count;
  size_t *pages_len;
} * book;

int book_create (book b);
#endif /* BOOK_HEADER_ */

/* book.c */
#include "book"

int
book_create (book b)
{
  b->pages_len = calloc (3, sizeof (b->pages_len));
  b->pages_len[2] = 20;
  return 0;
}

/* test.c */
#include "book.h"

int main(int argc, char** argv) {
  book my_book = (book)malloc (sizeof (book));
  int r = book_create (my_book);
  printf ("\n%lu\n", my_book->pages_len[2]);
  free (my_book->pages_len);
  free (my_book);
}

What I get from my memory leak detector is that free(my_book) gives a Memory corruption (written out of bounds?) error. One thing that fixed this error was changing the order of pages_count and pages_len but I don't know why.

I just typed the above example, so if there is any typo or syntactic error, please let me know. Thank you.

Answer 1

book my_book = malloc(sizeof(book)) is wrong. Note that the type of book is pointer to your struct, but you want to allocate enough space for the struct itself. So as the code stands, you will need to write malloc(sizeof(*my_book)).

However, using typedef to define a name for a pointer is usually bad style; it leads to confusion, and you will find it very awkward that your struct type doesn't have a name. A better way to write this would just be as

struct book
{
  size_t pages_count;
  size_t *pages_len;
};

struct book *my_book = malloc(sizeof(*my_book));

In this case malloc(sizeof(struct book)) would also work, but will become broken if you ever change the type of my_book.

It's often suggested that you not typedef away the struct, and just keep calling it struct book everywhere, because it's usually good to remember what kind of object you are working with. But if you must, you can still do typedef struct book book; afterwards. As mentioned, I would not recommend typedef struct book *book;.

Casting the result of malloc should never be necessary in C. You presumably included it because you got a warning about the return type of malloc, but the correct fix is to include the standard header <stdlib.h>. Indeed, this is precisely the reason why people usually recommend that you do not cast the result of malloc, because it can silence warnings that indicate a real bug. See Do I cast the result of malloc? for much more on this topic.

You also need <stdio.h> in test.c.

Also, create_book should return a value (but this is not the cause of your crash, since you never use it).

It is not surprising that rearranging the struct members made the bug appear to go away. What probably happened is something like this. On a typical 64-bit system, size_t and pointers are each 8 bytes. So you had allocated 8 bytes for your struct, when its size is actually 16. But you only actually wrote to the pages_len member. So if pages_len is the first member of the struct, and you never write to the pages_count member, you are only writing within the 8 bytes you allocated, and nothing goes wrong. Of course, the code was still broken, and as soon as you added any code that used the pages_count member, or added or rearranged any members of your struct, the bug would be back.

But in general, blindly changing things until a bug goes away is an extremely bad idea when programming in C. You may very easily do something that only masks the bug and makes it harder to find. There's no substitute for actually understanding what is happening.