Hunting down the reason for a crash...

[Steyr]

Hi everyone! this is my first post on cplusplus.com. I'm having some difficulty hunting down an error in the following code. It compiles fine, but when it runs it crashes... I'm guessing it's going into an infinite loop, but I can't find any logical errors in the code. The program is supposed to prompt for a number of students and names/test scores for each, and then sort the score records and print them out in ascending order, along with a class average. I'm using a selection sort, and I think that I wrote it wrong. Here's the code:

#include <iostream>
#include <string>
#include <iomanip>
using namespace std;

int main()
{
    int numscores;
    cout << "Please enter the number of test scores: ";
    cin >> numscores;

    struct scorerec
    {
        string name;
        double score;
    };

    scorerec *scoreset;
    scoreset = new scorerec[numscores];

    for(int z = 0; z < numscores; z++)
    {
        cout << "Enter name " << (z+1) << ": ";
        cin >> (*(scoreset + z)).name;
        cout << "Enter score: ";
        cin >> (*(scoreset + z)).score;

    };

for(int i = 0; i < numscores - 1; i++)  //I think the error is in this sort loop
{
    scorerec smallestrec;
    smallestrec = *(scoreset+i);
    int smallest = i;
    int x;
    for(x = i + 1; x < numscores; x++) 
    {
        if (((*(scoreset + x)).score) < (smallestrec.score))
        {
            smallestrec = (*(scoreset + x));
            smallest = x;
        };
    };
    *(scoreset + x) = *(scoreset + i);
    *(scoreset + i) = smallestrec;
}

    cout << "The scores for the class are:\n";
    for(int y = 0; y < numscores; y++)
    {
    cout << (*(scoreset + y)).name << setw(20) << (*(scoreset + y)).score << "\n";
    };
    double scoretotal = 0;
    for (int x = 0; x < numscores; x++)
    {
     scoretotal +=  (*(scoreset+x)).score;
    }
    cout << "The class average score is " << (scoretotal / numscores);
    return 0;
}

Any ideas?

[Moschops]

This line:

*(scoreset + x) = *(scoreset + i);

You end up with x being too big and trying to write off the end of the array. Here's some gdb output.

(gdb) run
Starting program: /home/j/badCode/a.out 
Please enter the number of test scores: 2
Enter name 1: a
Enter score: 34
Enter name 2: b
Enter score: 12

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b72238 in std::string::assign(std::string const&) () from /usr/lib/libstdc++.so.6
(gdb) up
#1  0x0000000000401901 in main::scorerec::operator= (this=0x604038)
    at /usr/include/c++/4.4.3/iomanip:242
242	    return __x; 
(gdb) up
#2  0x000000000040170f in main () at 218.cpp:44
44	    *(scoreset + x) = *(scoreset + i);
(gdb) print x
$1 = 2
(gdb) 

You can see that I provided two scores, but when the segFault happens, x is two, so *(scoreset + x) is trying to write to the third member of the array - but of course the array only has two elements in it.

[Catfish]

For every new, there must be a delete.
And the pointer must not change in between. (You are doing pointer arithmetic on it, use a temporary value.)

[andywestken]

You are going against convention. Almost all people would code the following

    scorerec *scoreset;
    scoreset = new scorerec[numscores];

    for(int z = 0; z < numscores; z++)
    {
        cout << "Enter name " << (z+1) << ": ";
        cin >> scoreset[z].name;
        cout << "Enter score: ";
        cin >> scoreset[z].score;

    } // no need for a ; here 

That is, having allocated an array, use array accessor notation.

Andy

P.S. You code as-is would fail code review where I work on that point. It makes the code harder than necessary to read!

[Steyr]

Ah, I see now.
*(scoreset + x) = *(scoreset + i);
should have been
*(scoreset + smallest) = *(scoreset + i);

Thanks for the help guys!