1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
|
$title "malloc, realloc and sizeof"
$tags information C
I spend a lot of time in ##c hoping to provide useful information to people
wishing to learn and understand C and almost every day I see malloc and realloc
code which appears to be straight from a horror movie. I've had to say what I
say in this post repeatedly to many people so I've decided that it's time to
write it once and simply give people a link every time I wish to explain why
such code is incorrect.
$pre
The code I'm describing is usually a combination of some or all of this:
~~~ c
struct foo *bar;
bar = (struct foo *)malloc(sizeof(struct foo) * 10);
bar = (struct foo *)realloc(bar, sizeof(struct foo) * 20);
~~~
There are multiple issues with this:
- Casting malloc can hide newbie mistakes such as forgetting to include
stdlib.h which can in turn cause strange issues down the line.
- Casting unnecessarily also gives you rather long lines, especially when you
have a rather complicated/long type (e.g. const struct foo * const *).
- You're having to type the same thing three times, when you could type it
once.
- If you screw up rewriting the same thing one of three times, you might end
up allocating the wrong amount of space and still end up with code which
runs, this can be dangerous down the line if some clever person finds a way
to exploit this.
- If you ever want to change the type of bar, you need to change it in 3
places, if you forget to change it in one of these, you end up with the same
issue described in the point above.
- Finally (regarding just the realloc line) if realloc fails, memory is leaked
since bar's original value is overwritten with NULL, realloc doesn't
automatically free the original pointer when it can't realloc.
The easy solution is simply:
~~~ c
struct foo *bar, *tmpbar;
bar = malloc(sizeof *bar * 10);
tmpbar = realloc(bar, sizeof *tmpbar * 20);
if (!tmpbar) {
free(bar);
/* error handling */
} else {
bar = tmpbar;
}
~~~
This solves all of the outlined problems.
-----------------------
Here's the answers to two common questions:
**Q**: _The compiler has always complained (warned) when I tried to assign a
pointer to one type to a pointer to another type. Would this not be the case
here?_
**A**: The C standard states (C11/C99 §6.3.2.3 and C11/C99 §6.5.16.1) roughly
that a pointer to void can be assigned to a pointer to any type, and vice
versa, freely without issuing a warning. This in turn means that the return
value of malloc (and any other function returning void \*) will be converted
without warning to the desired type without needing a cast.
**Q**: _What's the deal with `sizeof *bar`? Don't functions need
brackets?_
**A**: The sizeof operator can be given an expression as an operand as well as
the parenthesized name of a type. This means we can do `sizeof *bar`. Also,
since sizeof is an _operator_ and **not** a function, it doesn't require
parentheses when its operand is an expression, the C standard does however
require that for types, the type name has to be parenthesized. In the end, it's
a style choice whether the parentheses are always there or not, but personally I
think leaving them out is more consistent. (NB: The first example uses sizeof as
such: `sizeof(type)`, personally I prefer leaving a space between `sizeof` and
`(type)` as I would with most other operators.)
|
