To Do List, Reallocation in C

Question

I'm doing a homework assignment for Computing II. It's to create a to-do list of tasks in a dynamically created array of strings that can manipulated in a number of ways. One of the ways it needs to be manipulated is through the addition of a task, or an element of the array through the use of realloc. My code is as follows, and will run until I call the freshly realloc'd array in a different function.

void add_task(char **List, int line_num){
   char task[1000];

   List = (char**)realloc(List, (line_num+1)*sizeof(char));
   List[line_num] = malloc((1000) * sizeof(char));

   printf("Please enter the string you would like to use as your new task.\n");
   scanf("%s",task);
   strcat(task,"\n");
   strcpy(List[line_num],task);
   return;  
}

Do you have a question? I'd recommend ditching the `task[1000]`, you can `scanf()` directly into the `malloc()`ed buffer. Also, I'd return the new `line_num`, as you can be sure some caller will forget to increment his copy of it. — EOF, Jan 31 '15 at 01:10
[Don't cast malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Jan 31 '15 at 01:15
`sizeof(char)` is 1 by definition so you don't need it in the `malloc`. OTOH, in the `realloc` you meant to say `sizeof(char *)`, i.e. the sizeof of a pointer which is definitely not 1. The missing `*` is going to undersize the `List` by a factor of 4 or 8. — user3386109, Jan 31 '15 at 01:16
Thanks for the help EOF. With your changes I was able to call the array element successfully by itself, ie List[line_num]. I increment the line_num in main to eliminate the need of returning it. However, I still can't seem to use the new array element in a for loop, could this be because I'm incrementing in the main? I checked to make sure it worked right and the line_num did increment, but I'm not sure. I'll post more of my code in a few minutes. — Trevor King, Jan 31 '15 at 01:17
You should also limit the size of the string by using `scanf("%998s",task)` so that neither the `scanf` nor the following `strcat` can overrun your buffer. — user3386109, Jan 31 '15 at 01:19
C is call-by-value. `List = whatever` will not change the copy of `char ** List` in the caller. You need to either return the new `char **`, or add another level of indirection and do `*List = whatever`. — EOF, Jan 31 '15 at 01:20
@user3386109 solved it. Changing the `sizeof(char)` to `sizeof(char*)` fixed it. Thank you — Trevor King, Jan 31 '15 at 01:21
Unless you've also fixed the call-by-value issue, you're going to fail as soon as `realloc()` fails to resize in-place and returns a different address. — EOF, Jan 31 '15 at 01:25

score 1 · Answer 1 · answered Jan 31 '15 at 01:18

Your realloc() call is wrong, you're providing the wrong size. Since List is char**, the elements are char*, not char.

List = realloc(List, (line_num+1)*sizeof(char*));

Since sizeof(char*) is likely to be 4, you're allocating only 1/4 as much space as you need. And then you're writing outside the bounds of this array, resulting in undefined behavior.

In general, whenever you're assigning to <something>* with malloc or realloc, the argument to sizeof should be <something>, i.e. just remove the last * from the type.

To Do List, Reallocation in C

1 Answers1