malloc a 2D array, where each entry is a string, in C

Question

I am trying to malloc a 2dimensional array in C, where each entry is a string (so, I suppose, a 3dimensional array). I have read a lot, and this is my attempt. However, I am getting a segmentation fault and I'm really not sure what is wrong. I am quite new to programming, so I apologize if my style is not good!

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
int main(int argc, char *argv[])
{
 double temp, int_check;
 int gen, exit_flag=0, valid_input, valid, i, j;
 char ***S;
 printf("argc %d\n", argc);
 if(argc < 2)
 {
 printf("Please enter command line arguments of the form: a R where a is the number of generators and R are relators\n");
 exit_flag = 1;
 }
 else
 {
 valid = sscanf(argv[1], "%lg", &temp);
 int_check = temp - (int)temp;
 valid_input = ((valid != 0) && (valid != EOF)) && (int_check == 0) && (temp > 0);
 if(!valid_input)
 {
 printf("Invalid input, the number of generators must be an integer > 0\n");
 exit_flag = 1;
 }
 gen = (int)temp;
 printf("Number of generators = %d\n", gen);
 }
 if(exit_flag==0)
 {
 S = (char***)malloc(2*sizeof(char**)); /*Defintes the grid to the size required*/
 if(S == NULL) 
 {
 printf("Cannot allocate memory for the S");
 }
 for(i=0; i<2; i++)
 {
 S[i] = (char**)malloc((argc-2)*sizeof(char*));
 if(S[i] == NULL)
 {
 printf("Cannot allocate memory for the S");
 }
 } /*Grid finished being given the right size*/
 for(i=2; i<argc; i++) /*Put the relators in the grid. Make rhs equal 1*/
 {
 strcpy(S[0][i-2], argv[i]);
 strcpy(S[1][i-2], "1");
 printf("relator %s\n", S[0][i-2]);
 }
 printf("The array S is\n");
 for(j=0; j<(argc-2); j++)
 {
 for(i=0; i<2; i++)
 {
 printf(" %s ", S[i][j]);
 }
 printf("\n");
 }
 }
 else /*If the inputs are invalid, exit the program*/
 {
 exit(EXIT_FAILURE);
 }
 for(i=0; i<2; i++)
 {
 free(S[i]);
 }
 free(S);
 return 0;
}

Answer 1

You never allocate any memory for the actual characters, only for the various pointers pointing to those desired characters. Add the allocation here:

for(i=2; i<argc; i++) /* Put the relators in the grid. Make rhs equal 1*/
{
 S[0][i-2] = malloc(strlen(argv[i]) + 1); // allocate memory for the data
 strcpy(S[0][i-2], argv[i]);
 strcpy(S[1][i-2], "1");
 printf("relator %s\n", S[0][i-2]);
}

Don't forget to free the whole mess in the end.

I'm not sure the the repeated strcpys do what you want; strcpy adds a null terminator so your string ends there. Perhaps strncpy is a more useful function for your situation.

• user1163974: Thanks so much for your reply! This is really useful, I realise now that I need to assign memory for each character of the string.

Answer 2

Your problem is in lines like strcpy(S[0][i-2], argv[i]); . You didn't allocate space to this string, and it's garbage pointer. Use S[0][i-2]=strdup(argv[i]) instead.

• luser droog: <nit> strdup is POSIX, not "standard" C. </nit>
• asaelr: Thanks. I didn't remember that.

Answer 3

There are a lot of memory errors in that code. Sometimes you free without malloc'ing, other times you copy strings without checking their size (so the string might be too large for the destination), and you never actually allocate the memory for the strings.

• asaelr: No, he wants char ***S. It should be 2d array of strings.