Your code is unnecessarily complicated, because you have set things up such that:
n
: the number of words
words
: points to allocated memory that can hold n+1
char **
values in sequence
words[i]
(0 <= i && i < n
): points to allocated memory that can hold one char *
in sequence
words[n]
: NULL
words[i][0]
: points to allocated memory for a word (as before, 0 <= i < n)
Since each words[i]
points to stuff-in-sequence, there is a words[i][j]
for some valid integer j ... but the allowed value for j
is always 0, as there is only one char *
malloc()ed there. So you could eliminate this level of indirection entirely, and just have char **words
.
That's not the problem, though. The freeing loop starts with walker
identical to words
, so it first attempts to free words[0][0]
(which is fine and works), then attempts to free words[0]
(which is fine and works), then attempts to free words
(which is fine and works but means you can no longer access any other words[i]
for any value of i
—i.e., a "storage leak"). Then it increments walker
, making it more or less equivalent to &words[1]
; but words
has already been free()
d.
Instead of using walker
here, I'd use a loop with some integer i
:
for (i = 0; words[i] != NULL; i++) {
free(words[i][0]);
free(words[i]);
}
free(words);
I'd also recommending removing all the casts on malloc()
and calloc()
return values. If you get compiler warnings after doing this, they usually mean one of two things:
- you've forgotten to
#include <stdlib.h>
, or
- you're invoking a C++ compiler on your C code.
The latter sometimes works but is a recipe for misery: good C code is bad C++ code and good C++ code is not C code. :-)
Edit: PS: I missed the off-by-one lengthOfResult
that @David RF caught.