0

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]);
  }
}
5
  • 1
    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. Commented Nov 12, 2013 at 6: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. Commented Nov 12, 2013 at 7:05
  • If you use fgetsto read into the same variable several times in a row, what do you think will be stored? Commented Nov 12, 2013 at 7:07
  • Note that Three-Star Programmer is not altogether complimentary. Commented Nov 12, 2013 at 7:09
  • 1
    why the magic number "num+10" ? Commented Nov 12, 2013 at 7:16

2 Answers 2

2

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);
}
Sign up to request clarification or add additional context in comments.

1 Comment

stackoverflow.com/questions/19946208/… 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.
0

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.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.