C++ sorting stl vector fails (SIGTRAP)

Question

i try relatively simple task - create a vector and sort it. But unfortunately when destroying the vector from stack, it fails with SIGTRAP. When I remove the sort, its OK, but i rather need it.

I have narrowed the issue to smallest possible source code. When I run it, i get windows error "Program stopped unexpectedly". When i use GDB attached, it highlights the vector destructor after finishing the first while loop. (See picture below)

I have tried both vector<Vec> veci(n); and resize(n). I sense it could be caused by the sort algorithm somehow sorting items not present in the vector, but the code doesnt indicate the problem... do you see it?

Windows 10, mingw gcc

#include <stdio.h>
#include <cstdlib>
#include <fstream>
#include <iostream>
#include <ctime>
#include <stack>
#include <vector>
#include <algorithm>
#include <numeric>
#include <string>
#include <sstream>

using namespace std;

struct Vec {
    int cena;
    int vaha;
    float pomer;

    bool operator < (const Vec& oth) const
    {
        return !(pomer < oth.pomer);
    }
};

int main(int argc, char** argv) {

    stringstream file;
    file << "9550 40 600 14 223 38 230 3 54 1 214 13 118 4 147 15 16 2 104 5 56 49 154 40 106 24 234 18 34 33 195 7 74 10 129 12 159 42 37 41 10 11 185 6 243 45 87 32 57 20 87 9 26 16 201 39 0 23 128 39 194 21 10 46 1 8 28 30 59 26 130 35 160 22 91 34 180 19 16 31 1 17 72";

    while (file.good()) {
        int id;
        int n;
        int M;
        file >> id;
        file >> n;
        file >> M;
        vector<Vec> veci;
        veci.resize(n);
        for (int i = 0; i < n; i++) {
            int c, v;
            file >> v;
            file >> c;
            veci[i].vaha = v;
            veci[i].cena = c;
            veci[i].pomer = c / (v+1);
        }

        sort(veci.begin(), veci.end());

    }

    printf("end");
    return 0;
}

Answer 1

Your problem stems from using while (file.good()), which is virtually always an error (and is in this case).

This doesn't detect the end of file correct, and ends up writing the last item to the vector twice--but you've only made room for it to be written there once, so this writes past the end of the allocated space.

When the sort attempts to work with the data on the now-corrupted heap, you start to see the problem.

The correction is fairly trivial: read the data correctly, such as:

#include <stdio.h>
#include <cstdlib>
#include <fstream>
#include <iostream>
#include <ctime>
#include <stack>
#include <vector>
#include <algorithm>
#include <numeric>
#include <string>
#include <sstream>
#include <iterator>

using namespace std;

struct Vec {
    int cena;
    int vaha;
    float pomer;

    bool operator < (const Vec& oth) const {
        return !(pomer < oth.pomer);
    }

    friend std::istream &operator>>(std::istream &is, Vec &v) {
        return is >> v.vaha >> v.cena >> v.pomer;
    }

    friend std::ostream &operator<<(std::ostream &os, Vec const &v) {
        return os << "{" << v.vaha << ", " << v.cena << ", " << v.pomer << "}";
    }
};

int main(int argc, char** argv) {

    stringstream file;
    file << "9550 40 600 14 223 38 230 3 54 1 214 13 118 4 147 15 16 2 104 5 56 49 154 40 106 24 234 18 34 33 195 7 74 10 129 12 159 42 37 41 10 11 185 6 243 45 87 32 57 20 87 9 26 16 201 39 0 23 128 39 194 21 10 46 1 8 28 30 59 26 130 35 160 22 91 34 180 19 16 31 1 17 72";

    std::vector<Vec> veci { std::istream_iterator<Vec>(file), std::istream_iterator<Vec>{ } };

    sort(veci.begin(), veci.end());
    std::copy(veci.begin(), veci.end(), std::ostream_iterator<Vec>(std::cout, "\n"));
    return 0;
}

One final note: your comparison function works the reverse of what most people expect. It will also fail (completely) if you end up with a NaN in the pomer field of your struct (NaN's don't follow a strict weak ordering, as required by the comparison operator for std::sort).

Answer 2

The immediate problem - causing the SIGTRAP - is your comparison operator:

bool operator < (const Vec& oth) const
{
    return !(pomer < oth.pomer);
}

It's implemented as "! <", which is >=, so returns true for equal elements. An operator< must never return true for equal elements if it's to be used by std::sort. Requirements are summarised here.

You could instead say return power > oth.pomer.

Note that file.good() works correctly in your case, as after parsing the final 72 value there's no whitespace and extraction of that int will be terminated by eof, affecting the stream state. It's damned fragile though, as is the idea that because the stream state is good at any point in time, you can presume that future streaming operations work. It's better to test the stream state's still good at some point before trusting the variables you've attempted to stream into, especially if you're doing something like resize followed by array indexing.

Example of more robust I/O:

#define ASSERT(X, MSG) \
    do { \
        if (!(X)) { \
            std::cerr << "ASSERT fail @" << __LINE__ \
                << " !(" #X ") " << MSG << "\n"; \
            exit(EXIT_FAILURE); \
        } \
    } while (false)

while (file >> skipws && file.good())
{
    int id;
    int n;
    int M;
    ASSERT(file >> id >> n >> M, "unable to read 'id n m' fields");
    vector<Vec> veci;
    veci.resize(n);
    for (int i = 0; i < n; i++) {
        int c, v;
        ASSERT(file >> v >> c,
               "unable to read 'c v' fields for [" << i << ']');
        ASSERT(v + 1 != 0, "'v' value would trigger divide by zero");
        veci[i].vaha = v;
        veci[i].cena = c;
        veci[i].pomer = c / (v+1);
    }
}

Answer 3

Ok, the problem was veci[i].pomer = c / (v+1); assigning to float variable divison of two ints, thus solution is to use pomer = float(c) / (v+1).

Still, i think that the original code should just sort incorrectly. I dont understand why it fails completely, especially on vector's desctrutor... Anybody?

//edit: i dont know why it works now. But problem would be probably with the stream as @Jerry Coffin answered. I will react later.