Can anyone show me what is wrong here? Segmentation fault

I have to write a program that reads in a list of names of people and their age and then sort the names alphabetically. I have to use fgets() to read in each line and then use strtok() to partition the first name/last name/ages. Suppose the text file "names.txt" looks like this:

Ryan, Elizabeth 62
McIntyre, Osborne 84
DuMond, Kristin 18

This is my code for reading from this file:

Code:

void read_file()
{
  char **first_name;
  char **last_name;
  char line[80];
  FILE *fp = fopen("names.txt", "r");

  *first_name = malloc(count * sizeof(char)); //count is the number of names in file
  *last_name = malloc(count * sizeof(char)); //so in this case count = 3

  fgets(line, 80, fp);
  (*first_name)[0] = strtok(line, ", \n");
  printf("%s", (*first_name)[0]);
}

Right now I know it only reads the first line, I just want to check to make sure this works first. However, when I compile with gcc I get this message:

Code:

sort.c:23: warning: assignment makes integer from pointer without a cast

And when I run the program I get a segmentation fault. Can anyone tell me what is wrong with the code above? Any help is appreciated!

Originally Posted by BigFish21

I have to write a program that reads in a list of names of people and their age and then sort the names alphabetically. I have to use fgets() to read in each line and then use strtok() to partition the first name/last name/ages. Suppose the text file "names.txt" looks like this:

Ryan, Elizabeth 62
McIntyre, Osborne 84
DuMond, Kristin 18

This is my code for reading from this file:

Code:

void read_file()
{
  char **first_name;
  char **last_name;
  char line[80];
  FILE *fp = fopen("names.txt", "r");

  *first_name = malloc(count * sizeof(char)); //count is the number of names in file
  *last_name = malloc(count * sizeof(char)); //so in this case count = 3

  fgets(line, 80, fp);
  (*first_name)[0] = strtok(line, ", \n");
  printf("%s", (*first_name)[0]);
}

Right now I know it only reads the first line, I just want to check to make sure this works first. However, when I compile with gcc I get this message:

Code:

sort.c:23: warning: assignment makes integer from pointer without a cast

And when I run the program I get a segmentation fault. Can anyone tell me what is wrong with the code above? Any help is appreciated!

Why are those variables pointers-to-pointers? They should just be single pointers (1 asterisk).
The reason it's blowing up is because you are dereferencing an uninitialized pointer when calling malloc().

Originally Posted by cpjust

Why are those variables pointers-to-pointers? They should just be single pointers (1 asterisk).
The reason it's blowing up is because you are dereferencing an uninitialized pointer when calling malloc().

Perhaps I don't understand this concept completely yet, I thought I need the first_name and last_name variables to be pointers-to-pointers to create an array of strings? Because otherwise with just char *first_name wouldn't that just create an array of characters (a string)? I want to create 2 parallel arrays of strings, so that for example first_name[0] and last_name[0] represents the first person in the file (in my example above it would be Elizabeth Ryan. If I'm not understanding correctly please correct me! Thanks

Ok I figured out the array of strings part. Now I am trying to read in the different values using strtok() like my instructor said. I used a for loop like this:

Code:

for (i = 0; i < count; i++) {
  fgets(line, 80, fp);
  (*last_name)[i] = strtok(line, ", \n");
  (*first_name)[i] = strtok(NULL, ", \n");
  (*age)[i] = strtok(NULL, ", \n");
}

However, after this loop I use a simple printf() to check the values for (*last_name)[0], (*first_name)[0], and so on, and none of them are correct. However, when I don't use the for() loop and instead write out each statement separately all the values are what they should be. This works:

Code:

fgets(line, 80, fp);
(*last_name)[0] = strtok(line, ", \n");
(*first_name)[0] = strtok(NULL, ", \n");
(*age)[0] = strtok(NULL, ", \n");

fgets(line, 80, fp);
(*last_name)[1] = strtok(line, ", \n");
(*first_name)[1] = strtok(NULL, ", \n");
(*age)[1] = strtok(NULL, ", \n");

and so on

What's wrong with my loop?

If you know how many names are in the file, than there should be no reason to dynamically allocate a whole array of string. But none the less, you need to show us how you are allocating the array of strings, since that has changed since your last post. An isolated for loop does not help us debug your whole program. Remember that forum posters here aren't telepathic and can't guess at how you've changed things in your program, no matter how much you write about it, without seeing the changed code.

In regards to the array of strings:

You already have a variable like char **lastname; so for these types of variables you can do something like,
1. allocate char pointers for the number of strings in the array (the pointer to the whole array).
2. loop through all the allocated pointers, allocating one string of a certain length (say, 80) for each pointer.

And then you can access the strings like firstname[0], lastname[0], firstname[1], lastname[1] ... etc.

Then when you are finished with the string arrays,
1. loop through each string in the array, freeing the individual strings
2. then free the pointer to the whole array.

If you've done something different with the data structure than what I've suggested, then that may be the problem. We need to see.

Sorry about that, here's the entire program. But my instructor specifically said I need to dynamically allocate the strings based on a number on the first line of the file, so the actual "names.txt" file would read something like:

Code:

3
Ryan, Elizabeth 62
McIntyre, Osborne 84
DuMond, Kristin 18

Here's the program:

Code:

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

void read_file(char ***first_name, char ***last_name, char ***age)
{
  int count, i;
  char line[80];
  FILE *fp = fopen("names.txt", "r");

  fscanf(fp, "&#37;d", &count); //count = how many names in the file on the first line

  *first_name = (char **)malloc(count * sizeof(char));
  *last_name = (char **)malloc(count * sizeof(char));
  *age = (char **)malloc(count * sizeof(char));

  for (i = 0; i < count; i++)
  {
    (*first_name)[i] = (char *)malloc(20 * sizeof(char)); //teacher said no name will have more
    (*last_name)[i] = (char *)malloc(20 * sizeof(char)); //than 20 chars
    (*age)[i] = (char *)malloc(2 * sizeof(char));
  }

  for (i = 0; i < count; i++) //for() loop in question above
  {
    fgets(line, 80, fp);
    (*last_name)[i] = strtok(line, ", \n");
    (*first_name[i] = strtok(NULL, ", \n");
    (*age)[i] = strtok(NULL, ", \n");
  }

  printf("%s", (*last_name)[1]); //test if value is correct - its not
}

int main()
{
  char **age, **first_name, **last_name);
  read_file(&first_name, &last_name, &age);

  return 0;
}

This program isn't complete yet obviously, but I need to get the parts above correct before I can go on to how to sort them alphabetically by last name. If someone is willing to go through this and point out any mistakes that would be greatly appreciated, thanks for your time.

Whoa triple-pointers - You're just asking for trouble aren't you!

So you want an array of strings right? Well you're allocating some strings alright, but you're not properly allocating the array of pointers to put it into.

You're also never actually copying anything (use strcpy) into the strings you would have if you first fixed the above.

> *first_name = (char **)malloc(count * sizeof(char));
1. Remove the cast of the return result - see the FAQ on casting malloc.
2. The sizeof should be sizeof(char*).

> (*last_name)[i] = strtok(line, ", \n");
You've already malloc'ed the space, so you should be doing strcpy here, not reassigning the pointer.

Oh, so in C I should not include the cast on malloc()?

And so instead of doing this: (*last_name)[i] = strtok(line, ", \n");
I should do this: strcpy((*last_name)[i], strtok(line, ", \n"); ?

Does that work?

*edit*

Hm I get another dreaded segmentation fault when I changed my code. What am I doing wrong now? This whole pointers stuff is just really confusing right now.

Did you fix the sizeof on the first set of malloc calls?

Post your latest code. What you're trying to do is certainly possible (and you're very close), but there is no room for error.

Code:

void read_file(char ***first_name, char ***last_name, char ***age)
{
  int count, i;
  char line[80];
  FILE *fp = fopen("names.txt", "r");

  fscanf(fp, "%d", &count); //count = how many names in the file on the first line

  *first_name = (char **)malloc(count * sizeof(char*));
  *last_name = (char **)malloc(count * sizeof(char*));
  *age = (char **)malloc(count * sizeof(char*));

  for (i = 0; i < count; i++)
  {
    (*first_name)[i] = (char *)malloc(20 * sizeof(char)); //teacher said no name will have more
    (*last_name)[i] = (char *)malloc(20 * sizeof(char)); //than 20 chars
    (*age)[i] = (char *)malloc(2 * sizeof(char));
  }

  for (i = 0; i < count; i++) //for() loop in question above
  {
    fgets(line, 80, fp);
    strcpy((*last_name)[i], strtok(line, ", \n"));
    strcpy((*first_name[i], strtok(NULL, ", \n"));
    strcpy((*age)[i], strtok(NULL, ", \n"));
  }

}

I get a segmentation fault after compiling and running. I think the problem might be in the strcpy()? Am I allowed to do that with strcpy()?

You may want to check exactly where your seg fault is. Add a printf before each strcpy() to see which one is failing.

Another thought would be to have a temporar value, like this:

Code:

  for (i = 0; i < count; i++) //for() loop in question above
  {
    char *temp;
    static const char delim[] = ", \n";
    fgets(line, 80, fp);
    temp = strtok(line, delim);
    if (!temp) temp = "?last?";
    strcpy((*last_name)[i], temp);
    temp = strtok(NULL, delim);
    if (!temp) temp = "?first?";
    strcpy((*first_name[i], temp);
    temp = strtok(NULL, delim);
    if (!temp) temp = "??";
    strcpy((*age)[i], temp);
  }

By the way, your age element is 2 long, which means that only a single digit will actually fit - perhaps you should consider making it say 4 chars long - that works for ages up to 999, so should be OK.

And there is absolutely no reason why you can't allocate and read string in the same loop - using two loops is just making the code more complex for no useful reason.

--
Mats

To figure out exactly where you get a segfault, run your program from a debugger. It should tell you exactly where it happens. (You might have to get a backtrace -- type "backtrace" in GDB.)

Is this a copying error?

Code:

strcpy((*first_name[i], strtok(NULL, ", \n"));

You presumably want

Code:

strcpy((*first_name)[i], strtok(NULL, ", \n"));

[edit] Similar problem here.

Code:

char **age, **first_name, **last_name);

You should fclose() your file.

In addition, you shouldn't be casting malloc() in C, as Salem already mentioned. C allows the assignment of a void pointer to any other pointer, so a cast is completely unnecessary, and could hide a error like not including <stdlib.h>. It's only in C++ where you always need a cast to assign a void pointer to something. [/edit]

Ok I gave up on using strtok() because I just don't have enough time to finish the assignment right now. I hope I will be able to come back to it and get it later though. I just used a simple fscanf() for now and that worked fine.

Now I have another question, I want to pass the string arrays dynamically allocated in read_file() to another function called sort(). I tried declaring sort like this:

Code:

void sort(char ***first_name, char ***last_name, char ***age);

And then calling sort() in read_file() by doing: sort(first_name, last_name, age);
This worked fine it seemed. However, it makes sense to me that the sort() should look like this:

Code:

void sort(char **first_name, char **last_name, char **age)

and then making the same call to it in read_file() as I wrote above. This doesn't work, and I can't figure out why? I just want to pass the string array (which I'm pretty sure is a 2-dimensional array) to sort(), so I don't need to pass it the address of the pointers or anything. So it seems like just a char **first_name would suffice in the arguments. I get this warning when compiling with gcc:

Code:

sort.c:37: warning: passing argument 1 of 'sort' from incompatible pointer type
sort.c:37: warning: passing argument 2 of 'sort' from incompatible pointer type
sort.c:37: warning: passing argument 3 of 'sort' from incompatible pointer type

Can anyone explain to me why this is? Thanks

Originally Posted by BigFish21

I tried declaring sort like this:

Code:

void sort(char ***first_name, char ***last_name, char ***age);

And then calling sort() in read_file() by doing: sort(first_name, last_name, age);
This worked fine it seemed. However, it makes sense to me that the sort() should look like this:

Code:

void sort(char **first_name, char **last_name, char **age)

and then making the same call to it in read_file() as I wrote above. This doesn't work, and I can't figure out why? I just want to pass the string array (which I'm pretty sure is a 2-dimensional array) to sort(), so I don't need to pass it the address of the pointers or anything. So it seems like just a char **first_name would suffice in the arguments. I get this warning when compiling with gcc:

Code:

sort.c:37: warning: passing argument 1 of 'sort' from incompatible pointer type
sort.c:37: warning: passing argument 2 of 'sort' from incompatible pointer type
sort.c:37: warning: passing argument 3 of 'sort' from incompatible pointer type

Can anyone explain to me why this is? Thanks

Now you're up to a 3-dimensional array of characters!
Each * is one dimension, so each of these pairs are basically equivalent:

Code:

char* array = "1 dimension";
char array[] = "1 dimension";

char** array2D = { "1 dimension", "2 dimension" };
char array2D[][] = { "1 dimension", "2 dimension" };

char*** array3D = { {"1 dimension", "2 dimension" }, {"3 dimension", "3 dimension"} };
char array3D[][][] = { {"1 dimension", "2 dimension" }, {"3 dimension", "3 dimension"} };