20

I have a struct defined in .h

struct buf_stats {
   // ***
};

then in .c file

struct buf_stats *bs = malloc(sizeof(struct buf_states*)) ;

where buf_states is a typo.

but gcc does not warn me, although I used -Wall

and this bug/typo cost me 3 hours to find out.

How to make gcc warn undefined struct like this?

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
Sato
  • 8,192
  • 17
  • 60
  • 115
  • Heh? What was the error message you got? – Sourav Ghosh Sep 05 '17 at 07:47
  • @SouravGhosh I think there was NO error message and that is what OP is asking about. – Yunnosch Sep 05 '17 at 07:48
  • 17
    Dou want to malloc enough space for a pointer? Compiler does not really need the size of what it is pointing to for that. Or do you want to malloc for a struct? In that case, there is another typo, the second `* `. – Yunnosch Sep 05 '17 at 07:49
  • 2
    You may even write `struct XXX *bs = malloc(sizeof(struct YYY*));` and you still don't get any error. Only when you write `bs->something` you'll get a compilation error. – Jabberwocky Sep 05 '17 at 07:51
  • 4
    It would still be wrong even if you didn't make the typo – M.M Sep 05 '17 at 08:21
  • As others explained there are legitimate reasons to use pointers to opaque `struct`-s. If you want to be warned, you could spend weeks to write your GCC plugin (or your [GCC MELT](http://gcc-melt.org/) extension) to check that. Are you sure it is worth spending that much efforts? 3 hours wasted is not that much (and with a code review could be less). – Basile Starynkevitch Sep 05 '17 at 11:41

2 Answers2

35

In your code

  struct buf_stats *bs = malloc(sizeof(struct buf_states*)) ;

is wrong for many reasons, like

  • You are using an undefined type (as you mentioned)
  • You are allocating way less memory (allocating for a pointer-to-type instead of the type)

But you compiler can't help much in _this_case for this particular type of error, as

  • a pointer to (any) type in a platform has a defined size, for that the structure (i.e. the type of the variable to which it points to) need not be complete (defined). This is the reason we can have self-referencing structures, right?

  • malloc() has no idea about the target variable type. It just reads the argument for the needed size, return a pointer (which is of type void *) to the allocated memory and upon assignment, that gets changed to the target type. It cannot possibly calculate the mismatch in the target size (type) with the allocated memory size.

Most convenient and simplest way to avoid these type of mistakes is, not to use the hard-coded type directly as the operand of sizeof, rather, use the variable reference.

Something like

 struct buf_stats *bs = malloc(sizeof *bs) ; // you can write that as (sizeof (*bs)) also
                                             // sizeof *bs === sizeof (struct buf_stats)

which is equivalent to

 struct buf_stats *bs = malloc(sizeof(struct buf_stats)) ;

but is more robust and less error prone.

Notes:

  1. You don't need the parenthesis if the operand is not a type name.
  2. This statement does not need any modification upon changing the type of target variable bs.
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • 2
    It is probably worth mentioning that the `*` in `sizeof *bs` does *not* indicate a pointer, but a dereference. `bs` is a pointer, so `*bs` is the struct value itself, on which sizeof is called here. – tomsmeding Sep 05 '17 at 11:21
  • In addition, note 1 is good because it actually will produce an error similar to what @Sato is asking for, in the event that either `struct but_stats` is not complete, or `bs` is a type name instead of a variable name. – Alex Celeste Sep 05 '17 at 12:18
  • 1
    I've seen platforms for which pointers to different-sized things were different sizes. (void * was the largest pointer to any data type to make this work.) – Joshua Sep 05 '17 at 14:34
  • @Joshua: in general pointers to different types can have different representations (including sizes), but there are exceptions: types that are compatible except for qualifiers must be the same, `void*` and all `char*` ditto, all `struct*`, and all `union*`. See 6.2.5p28 in C11 (n1570) or p26 in C99. – dave_thompson_085 Sep 05 '17 at 18:49
  • @dave_thompson_085: I used one where sizeof(char *) == 2 while sizeof(const char *) == 3! Amusingly; (char *)(const char *)char_pointer worked anyway even when the cast couldn't be optimized out. – Joshua Sep 05 '17 at 19:18
17

You can't. Using an expression like struct foo * (a pointer to some struct type) declares that struct as an incomplete type. A size isn't known, but it's not necessary for the size of the pointer.

That said, the code looks wrong, as you need the size of the struct (not the size of the pointer), so with the following code:

struct buf_stats *bs = malloc(sizeof(struct buf_states));

you would get an error.

There's a better way to write such code:

struct buf_stats *bs = malloc(sizeof *bs);

The expression *bs has the correct type for sizeof, even when you later change the type.