Can anyone show me what is wrong here? Segmentation fault

This is a discussion on Can anyone show me what is wrong here? Segmentation fault within the C Programming forums, part of the General Programming Boards category; I have to write a program that reads in a list of names of people and their age and then ...

  1. #1
    Registered User
    Join Date
    Nov 2007
    Posts
    39

    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!

  2. #2
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by BigFish21 View Post
    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().

  3. #3
    Registered User
    Join Date
    Nov 2007
    Posts
    39
    Quote Originally Posted by cpjust View Post
    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
    Last edited by BigFish21; 11-20-2007 at 09:36 PM.

  4. #4
    Registered User
    Join Date
    Nov 2007
    Posts
    39
    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?

  5. #5
    Registered User whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    7,762
    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.

  6. #6
    Registered User
    Join Date
    Nov 2007
    Posts
    39
    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.

  7. #7
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    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.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  8. #8
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,855
    > *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.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  9. #9
    Registered User
    Join Date
    Nov 2007
    Posts
    39
    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.
    Last edited by BigFish21; 11-21-2007 at 01:32 AM.

  10. #10
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,855
    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.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  11. #11
    Registered User
    Join Date
    Nov 2007
    Posts
    39
    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, "&#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);
        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()?

  12. #12
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  13. #13
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    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]
    Last edited by dwks; 11-21-2007 at 03:15 PM.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  14. #14
    Registered User
    Join Date
    Nov 2007
    Posts
    39
    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

  15. #15
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by BigFish21 View Post
    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"} };

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Segmentation fault problem
    By odedbobi in forum Linux Programming
    Replies: 1
    Last Post: 11-19-2008, 03:36 AM
  2. Why am I getting segmentation fault on this?
    By arya6000 in forum C++ Programming
    Replies: 6
    Last Post: 10-12-2008, 07:32 AM
  3. segmentation fault... first time with unix...
    By theMethod in forum C Programming
    Replies: 16
    Last Post: 09-30-2008, 03:01 AM
  4. Segmentation fault
    By NoUse in forum C Programming
    Replies: 4
    Last Post: 03-26-2005, 03:29 PM
  5. Locating A Segmentation Fault
    By Stack Overflow in forum C Programming
    Replies: 12
    Last Post: 12-14-2004, 01:33 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21