Free() inside a while loop

Question

I'm trying to run this program where a character array is created and allocated memory dynamically. Later the array elements are populated with the string "hello" for 10 consecutive locations. The values are assigned to the string elements using strdup() function call.

Once all the elements are assigned the elements are freed up in the while loop.When I run the program in the Visual Studio, the program crashes after the last pointer to the char array is freed. I believe the termination condition for the while loop is correct. But I'm not able to identify what exactly is causing the issue.

Code:

char **p;
int i;

p = malloc(10 * sizeof(char *));
for (i = 0; i < 10; i++) {
    p[i] = strdup(“hello”);
}

while (*p) {
 free(*p++);
}

`*p` being of type pointer, won't be evaluated as *false* unless it's `NULL`. `*p` won't magically evaluate to `NULL` when you get out of bounds of the array `p`. However you can make it work by taking an extra measure yourself, i.e. `p = malloc(11 * sizeof * p); p[10] = NULL; /* rest shall be as in yours */` — Utkan Gezer, Oct 07 '14 at 06:44

Vlad from Moscow · Accepted Answer · 2015-01-04T17:33:57.917

4

If you want very much to use your while loop then you should write the code the following way

char **p;
int i;

p = malloc(11 * sizeof(char *));
for (i = 0; i < 10; i++) {
    p[i] = strdup(“hello”);
}

p[i] = NULL;

while (*p) {
 free(*p++);
}

Take into account that you need also to free the initial value of p itself. So the correct code with the while loop could look like

char **p;
int i;

p = malloc(11 * sizeof(char *));
for (i = 0; i < 10; i++) {
    p[i] = strdup(“hello”);
}

p[i] = NULL;

char **q = p;

while (*q) {
 free(*q++);
}

free( p );

edited Jan 04 '15 at 17:33

answered Oct 07 '14 at 06:46

Vlad from Moscow

301,070
26
186
335

Exactly: the free-ing loop is expecting a _sentinel_ in the array of pointers, which the original allocating loop never writes. Your p[i] = NULL statements writes the sentinel. – John Källén Oct 07 '14 at 06:50
Shouldn't `q` be declared as `char **` instead of `int **`? I know the pointers are the same, but for consistency... – xOneca Jan 04 '15 at 17:22

NPE · Answer 2 · 2014-10-07T06:55:47.330

1

You should be iterating over ten elements, not until the non-existent sentinel value (NULL):

for (i = 0; i < 10; i++) {
    free(p[i]);
}

Your current code dereferences p[10], which is outside the array bounds and therefore triggers undefined behaviour.

edited Oct 07 '14 at 06:55

answered Oct 07 '14 at 06:32

NPE

486,780
108
951
1,012

score 1 · Answer 3 · answered Oct 07 '14 at 06:36

1

Accessing values outside of array bound, means un authorized memory access. So only,You got crashes at end because of p++ . So try like this.

  i=0;
  while(i<10)
  {
   free(p[i]);
   i++;
  }

answered Oct 07 '14 at 06:36

Anbu.Sankar

1,326
8
15

Free() inside a while loop

3 Answers3