How to strcpy strings into a two dimension array

Question

I run the program in unix. It had segmentation faults. I found out it is from the for loop in read_names function. When I disabled the loop and set i = 0, it worked. However, when I set i = 1 or other number, it showed segmentation fault again. I think the way I store string might be wrong. Could anyone help me figure out this problem?

Besides, could I use strtok to save strings into a two dimension array?

Thank you.

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

void alloc(char ***surname, char ***first, char **mid_init, int num);
void read_names(FILE *inp, char ***surname, char ***first, char **mid_init, int num );
void free_memory(char ***surname, char ***first, char **mid_init, int num);

int main(int argc, char *argv[])
{
  int num;
  char **surname, **first, *mid_init;
  FILE *inp = fopen(argv[1], "r");  
  FILE *outp = fopen(argv[2], "w");
  char array[79];

  fgets(array, 79, inp);
  fgets(array, 79, inp);
  fgets(array, 79, inp);
  printf("%s", array);

  fscanf(inp, "%d", &num);
  fprintf(outp, "%d \n\n", num+10);

  alloc(&surname, &first, &mid_init, num);
  read_names(inp, &surname, &first, &mid_init, num);
  free_memory(&surname, &first, &mid_init, num);



  fclose(inp);
  fclose(outp);

  return 0;
}

void alloc(char ***surname, char ***first, char **mid_init, int num)
{
  int i;

  *surname = (char**)malloc(num * sizeof(char*));
  *first = (char**)malloc(num * sizeof(char*));
  *mid_init = (char*)malloc(num * sizeof(char));

  for(i=0; i<num; i++)
  {
    (*surname)[i] = (char*)malloc(15*sizeof(char));
    (*first)[i] = (char*)malloc(10*sizeof(char));
  }
}

void read_names(FILE *inp, char ***surname, char ***first, char **mid_init, int num )
{
  char *token, array[79];
  char delim[6] = ", .\n";
  int i=0/*, k=0*/;

  printf("loop begins\n");

  for(i=0; i<num; i++)
  {
      fgets(array, 79, inp);
      printf("%s\n", array);

      fgets(array, 79, inp);
      printf("%s\n", array);

      token = strtok(array, delim);
      strcpy( *(*(surname+i)+0), token);
      printf("%s   ", *(*(surname+i)+0));

      token = strtok(NULL, delim);  
      strcpy( *(*(first+i)+0), token);
      printf("%s  ", *(*(first+i)+0));

      token = strtok(NULL, delim);
      **mid_init = token[0];
      printf("%s\n", *mid_init);


  }
     printf("\nloop ends\n");
}

void free_memory(char ***surname, char ***first, char **mid_init, int num)
{
  int i;

  free((*mid_init));

  for(i=0;i<num;i++)
  {
    free((*surname)[i]);
    free((*first)[i]);
  }
}

Build with debug information (`gcc -g`). Run your code in a debugger (`gdb`). It will tell you the line of code where your program is crashing. — Jonathon Reinhart, Nov 12 '13 at 06:21
I have not completely read your program. Just giving you a quick hint to check you allocated sufficient memory using malloc. For example if an array contains a name "MyName" then you have to allocate length_of_str+1 to store the string. If you allocate memory whose size is only equal to length_of_str you may get the segmentation fault. I faced this issue and hence want you to have a quick look at your code to make sure this is done properly. — Anand, Nov 12 '13 at 07:05
If you use `fgets`to read into the same variable several times in a row, what do you think will be stored? — jpw, Nov 12 '13 at 07:07
Note that [Three-Star Programmer](http://c2.com/cgi/wiki?ThreeStarProgrammer) is not altogether complimentary. — Jonathan Leffler, Nov 12 '13 at 07:09

score 2 · Accepted Answer · edited May 23 '17 at 10:25

Your read function should be defined as:

void read_names(..., char **surname, char **first, ...)

You pass ***surname pointer to alloc() function because you are changing the outer variable surname. Read function doesn't do that, it accesses memory.

Then replace this scary :) code:

strcpy( *(*(first+i)+0), token);

with

strcpy(*(first+i), token);

more readable (as suggested by alk)

strcpy(first[i], token);

Actually for readability I'd suggest using char *pointer[]; declarations.

You use strtok to separate a string into tokes. Use strcpy with strtok to fill in your 2 dimensional array.

Also, you have a memory leak.

You need to add in free_memory() function

free(*surname);

But it can be simplified by changing free_function definition

void free_memory(char **surname, ..., int num)
{
  ...
  for(i=0;i<num;i++)
  {
    free(surname[i]);
    ...
  }
  free(surname);
}

http://stackoverflow.com/questions/19946208/free-caused-segmentation-fault Hi dmitri, I did follow your suggestion. The program can print out all the string from the file right now. However, there is a segmentation fault appeared after "loop ends" which is in read_name function. I could not figure out any problem in free_memory. Could you help me figure it our? Many thanks to that. — Bruce, Nov 13 '13 at 05:45

score 0 · Answer 2 · edited May 23 '17 at 10:31

First of all, the code misses error checking for all relevant system calls:

fopen()
fgets()
fscanf()
malloc()

Also do not cast the result of malloc/calloc/realloc as in C it is not necessary or recommanded.

Now for the major issues:

Change

strcpy( *(*(surname+i)+0), token);

to be

strcpy((*surname)[i]), token);

The same for how first is used.

This line

**mid_init = token[0];

does not make sense as you save a pointer to memory allocared local to the function which is invalid as soon as the function had been left.

In free_memory() add

free(*surname);
free(*first);

after the loop to avoid a memory leak.

How to strcpy strings into a two dimension array

2 Answers2