-3

(Segmentation fault) Error occurs while accessing the value associated with the key in the unordered map. Any pointers a where I am making the mistake?

Segmentation fault in the following snippet of code:

void read_mdl(const std::string& fname) {
        std::ifstream f(fname);

        std::unordered_map<int, std::vector<double>> pt_;

        while(f) {
            int xi = -1;
            int pa = 0;
            double score = -1;
            f >> xi >> pa >> score;
            if(xi == -1) { break; }

            std::unordered_map<int, std::vector<double>>::iterator it = pt_.find(pa);

            std::cout << xi << " " << pa << " " << score << std::endl;

            if (it != pt_.end()) {
                // Update from @Matthias247
                 auto a = pt_.insert(std::make_pair(pa, std::vector<double>(n_, 0)));
                 it = a.first;
            }

            (it->second)[xi] = score; // causing segmentation fault
        }
    }
letsBeePolite
  • 1,831
  • 1
  • 13
  • 31
  • 3
    Think about what will happen when `it` == `pt_.end()`. – songyuanyao Jan 28 '17 at 12:31
  • 1
    Your way to check for no more data from the stream is flawed. See e.g. [this old answer of mine](http://stackoverflow.com/a/13379073/440558) as for why. Also read ["Why is iostream::eof inside a loop condition considered wrong?"](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). Use e.g. `while (f >> xi >> pa >> score)` instead. – Some programmer dude Jan 28 '17 at 12:32
  • Also consider what will happen if `xi >= n_`. – G.M. Jan 28 '17 at 12:46
  • @G.M. that condition is guaranteed to not occur – letsBeePolite Jan 28 '17 at 13:01

2 Answers2

0

Your iterator is broken in the case where it doesn't find the element. You insert a new element in this case with pt_.insert but don't change the iterator. Accessing it afterwards leads to the segfault. Either do a new find or lookup if insert returns an iterator and assign that to it.

Update:

If I understand the insert API correctly it should work like

if (it != pt_.end()) {
    auto a = pt_.insert(std::make_pair(pa, std::vector<double>(n_, 0)));
    it = a.first;
}

(it->second)[xi] = score;
Matthias247
  • 8,111
  • 1
  • 14
  • 25
0

Found the bug.

It was in the incorrect checking of whether the key exists or not.

With respect to my snippet:

if (it != pt_.end()) // incorrect way

if (it == pt_.end()) // right way
letsBeePolite
  • 1,831
  • 1
  • 13
  • 31