Odtnhj

I've got an array of strings

char **tab;

and I want it to be able to increase as user types in new strings. I've created this code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(int argc, char *argv)

 char **tab=malloc(sizeof(char*));
 tab[0]=malloc(41*sizeof(char));
 int i=0;
 int d=0;
 while(strcmp(tab[i],"END"))
 
 if(d==0)
 
 i--;
 d++;
 
 i++;
 scanf("%s",tab[i]);
 **tab=realloc(tab,(i+2)*sizeof(char*));
 tab[i+1]=malloc(41*sizeof(char));
 
 return 0;

its purpose is to read new strings to tab untill user writes keyword "END". Since it's not known how many words will be inputted I tried to reallocate the size of array after each iteration. Unfortunately it prints Segmentation fault after getting 3 words.
What did i do wrong? Is there any better way to do this task?

Maximum length of any word is 40

Your algorithm is broken in two significant places

• 
  **tab = ... in the loop body is wrong. If tab is char**, then *tab would be char*, and **tab would be char. Assigning a memory address to char should flag huge warnings from your toolchain, and if it doesn't either turn up your warning levels or get a new toolchain.


• Initial entry to your while condition evaluates indeterminate data. At that time you have allocated raw memory for tab[0], but nothing has populated it yet. Therefore, your program invokes undefined behavior.

Besides the above, expansion algorithms aren't complex, and explaining yours to your rubber-duck will help significantly before writing any code. In doing so, you'll see reallocating a pointer array with each new read is both costly and inefficient. A geometric expansion algorithms makes this much better. As a bonus, it fixes both of the problems above.

Code

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_BUFLEN 41
#define MAX_STR_FMT "%40s"

int main()

 char **tab = NULL;
 size_t size = 0, capacity = 0;

 char str[MAX_BUFLEN];

 while (scanf(MAX_STR_FMT, str) == 1 && strcmp(str, "END"))
 
 // check for expansion 
 if (size == capacity)
 
 size_t new_capacity = 2 * capacity + 1;
 void *tmp = realloc(tab, new_capacity * sizeof *tab);
 if (tmp == NULL)
 
 perror("Failed to expand dynamic table");
 break;
 

 // save expanded table, and update capacity
 tab = tmp;
 capacity = new_capacity;
 

 size_t slen = strlen(str)+1;
 if ((tab[size] = malloc(slen)) == NULL)
 
 perror("Failed to allocate buffer for new string");
 break;
 

 // copy incoming string; update 'size' to reflect new count
 memcpy(tab[size++], str, slen);
 

 //
 // TODO: use 'tab' holding 'size' pointers. 
 //

 // then free the table
 while (size-- > 0)
 free(tab[size]);
 free(tab);

 return 0;

Alternative: No pointers to pointers

If your need truly requires fixed length allocations (as your code demonstrates), you don't need pointers to pointers at all (unless there is some hidden agenda for something like sorting where swapping pointers is much more efficient than swapping full string buffers).

Code

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_BUFLEN 41
#define MAX_STR_FMT "%40s"

int main()

 char (*tab)[MAX_BUFLEN] = NULL; // see difference here
 size_t size = 0, capacity = 0;

 char str[MAX_BUFLEN];

 while (scanf(MAX_STR_FMT, str) == 1 && strcmp(str, "END"))
 
 // check for expansion 
 if (size == capacity)
 
 size_t new_capacity = 2 * capacity + 1;
 void *tmp = realloc(tab, new_capacity * sizeof *tab);
 if (tmp == NULL)
 
 perror("Failed to expand dynamic table");
 break;
 

 // save expanded table, and update capacity
 tab = tmp;
 capacity = new_capacity;
 

 // notice no additional allocations here
 strcpy(tab[size++], str);
 

 //
 // TODO: use 'tab' holding 'size' strings. 
 //

 // then free the table
 free(tab);

 return 0;

Summary

Fixing your code was fairly simple, but making it better is too. Never stop thinking about why you're doing what your doing when crafting your algorithms, and spend plenty of time talk with your rubber-duck.

The problem lies in this line:

**tab=realloc(tab,(i+2)*sizeof(char*));

Here **tab dereferences tab to the first character in the first string stored in tab, which isn't what you want. Try this instead:

tab=realloc(tab,(i+2)*sizeof(char*));

If you turn on compiler warnings, they should catch this type of mistake.