Thread: I get this error Segmentation fault in my program when I run it, and I don't know why

  1. #1
    Registered User
    Join Date
    Sep 2018
    Posts
    31

    I get this error Segmentation fault in my program when I run it, and I don't know why

    Code:
    #include <stdlib.h>
    void cntSort(unsigned m, unsigned n, unsigned data[])
    {
      int i;
      unsigned temp[m];
      unsigned *count;
    
      /* allocate memory */
      count = (unsigned *) malloc(sizeof(unsigned) * m);
      for (i = 0; i < n; i++)
        count[temp[m]]++;
      for (i = 0; i < m; i++)
        temp[count[data[i]]]++;
      for (i = 0; i < n; i++)
        data[i] = temp[i];
      /* free up memory */
      free(count);
    
    }
    ./cntSort -m 64 -n 1048676 -s2018
    m 64, n 1048676, seed 2018
    quicksort takes 78.00 msec

    Segmentation fault
    Last edited by Salem; 10-23-2018 at 10:05 PM. Reason: Removed crayola

  2. #2
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    Where is temp[] ever initialized?

    Do you realize that temp[m] is outside the bounds of the array?

    I'm also curious about the contents of data[] and the size of data[].

  3. #3
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by jimblumberg View Post
    Where is temp[] ever initialized?
    Or count[], for that matter.

    Also, for a function that has "sort" in the name, there doesn't seem to be a lot of that going on. You do completely clobber your data vector, though, so that's something.

  4. #4
    Registered User
    Join Date
    Dec 2017
    Posts
    1,626
    Code:
    void cntSort(unsigned m, unsigned n, unsigned data[])
    {
        unsigned temp[m];                                    // uninitialized VLA
        unsigned *count = malloc(m * sizeof(*count));        // uninitiailzed dynamic array
        for (int i = 0; i < n; i++)  count[temp[i]]++;
        for (int i = 0; i < m; i++)  temp[count[data[i]]]++;
        for (int i = 0; i < n; i++)  data[i] = temp[i];
        free(count);
    }
    Your code makes no sense. You're using the uninitialized values of temp to access the uninitialized values of count and increment them.

    I think you want something more like this:
    Code:
    void cntSort(unsigned *data, unsigned size, unsigned maxValue) {
        unsigned *count = calloc(maxValue + 1, sizeof *count);
        for (unsigned i = 0; i < size; i++) count[data[i]]++;
        for (unsigned val = 0, i = 0; val <= maxValue; val++)
            for (unsigned j = 0; j < count[val]; j++)
                data[i++] = val;
        free(count);
    }
    Last edited by john.c; 10-23-2018 at 03:15 PM.
    A little inaccuracy saves tons of explanation. - H.H. Munro

  5. #5
    Registered User
    Join Date
    Sep 2018
    Posts
    31
    I comment out the temp[]

    still shows the same error

  6. #6
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by erald23 View Post
    I comment out the temp[]

    still shows the same error
    If you comment out the temp[] ... what's left? You use it on every single line of the function that does something (i.e., not the for-loop headers), other than the malloc/free.

  7. #7
    Registered User
    Join Date
    Sep 2018
    Posts
    31
    So what do you suggest to put instead because I'm confused with this error, plus I'm pretty new at C Programming language

  8. #8
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    The first thing you need to do is figure out what that function is supposed to do, right now I have no idea what it should be doing so you need to explain to me what you need to do.

  9. #9
    Registered User
    Join Date
    Sep 2018
    Posts
    31
    Counting sort assumes the elements are integers between 0 and m, and m is much smaller than
    n. For example, let m be 3, and n be 10. Then the elements can be 0, 1, or 2, and we have ten of
    them.
    Code:
    unsigned data[10] = {0, 2, 1, 1, 0, 2, 2, 0, 1, 1};
    unsigned count[3];
    Counting sort uses an array unsigned count[m] and initializes all elements in count to zero. Then
    we scan the array data. When we see a number data[i], this number is used as the index to the
    array count, and the corresponding count is incremented. After we finish scanning the array data,
    we have count[0] == 3, count[1] == 4, and count[2] == 3. How do we construct the sorted
    array from these counts? We write 0 for count[0] times, 1 for count[1] times, and 2 for count[2]
    times.

  10. #10
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    The first thing you need to do is: "Counting sort uses an array unsigned count[m] and initializes all elements in count to zero." Don't forget the highlighted section.

    Next: "Then we scan the array data. When we see a number data[i], this number is used as the index to the array count, and the corresponding count is incremented."

    After we finish scanning the array data,
    we have count[0] == 3, count[1] == 4, and count[2] == 3. How do we construct the sorted
    array from these counts? We write 0 for count[0] times, 1 for count[1] times, and 2 for count[2]
    times.
    Sounds about right to me.

  11. #11
    Registered User
    Join Date
    Sep 2018
    Posts
    31
    So what's wrong with my code then if you say sound about right?

  12. #12
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by erald23 View Post
    So what's wrong with my code then if you say sound about right?
    He's saying that sounds like a reasonable thing to do, not that you were doing anything like that in your code.

    The bold part has been pointed out more than once, and the quoted part is pretty much a statement-by-statement description of what your code needs to do, so you should do those things. john.c is apparently psychic, as he figured out that's what you wanted, so that might give you an idea of what the structure would have to be (a loop that goes through the possible values 0, 1, 2, and inside that a loop that writes the values to the data vector the appropriate number of times).

  13. #13
    Registered User
    Join Date
    Sep 2018
    Posts
    31
    Quote Originally Posted by tabstop View Post
    He's saying that sounds like a reasonable thing to do, not that you were doing anything like that in your code.

    The bold part has been pointed out more than once, and the quoted part is pretty much a statement-by-statement description of what your code needs to do, so you should do those things. john.c is apparently psychic, as he figured out that's what you wanted, so that might give you an idea of what the structure would have to be (a loop that goes through the possible values 0, 1, 2, and inside that a loop that writes the values to the data vector the appropriate number of times).
    Thank you in advance for helping me, I believe for you guys is something easy but I'm a beginner in C and struggling, I try to do this so far
    Code:
    for (i=0; i<m-1; i++) {
        count[i];
       temp= data[i]
    for (j=0; j< n-1; j++){
    if (data[j] = temp){
    printf"count[i]"
    count[i++];
    
    }
    
     }
    
     }
    Last edited by erald23; 10-23-2018 at 05:06 PM.

  14. #14
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Can you write down any of the instructions you were given, just by itself? (I'm assuming, just based on the difference in language, that your fourth post is the directions of the assignment verbatim.) We've seen

    • Setting each member of an array to zero
    • Use data[i] as the index to the array count, and increment the count
    • Write 0 count[0] times into a data array
    • Do the above in a loop to write all the values out

    Once you can do each of those things, then put them all in the same place.

  15. #15
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    Let's start by having you post the entire program, not just a few bits and pieces.

    Next do you know how to print that data[] array? If so show us a program that prints that array.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. segmentation fault error [help]
    By blaze505 in forum C Programming
    Replies: 9
    Last Post: 10-21-2011, 11:38 PM
  2. GCC Segmentation fault error.
    By JPinUSMC in forum C Programming
    Replies: 2
    Last Post: 02-21-2011, 10:59 PM
  3. Replies: 24
    Last Post: 02-09-2011, 09:19 AM
  4. Segmentation fault Error
    By unknown_ in forum C Programming
    Replies: 7
    Last Post: 03-21-2010, 01:32 PM
  5. Segmentation Fault Error
    By jcramer in forum C Programming
    Replies: 2
    Last Post: 11-23-2003, 02:16 PM

Tags for this Thread