-1

Can somebody tell me what is the problem with this program? The idea is to display "Yes" if the vector array satisfies all these criteria: The array elements are not sorted in ascending order. The array contains distinct elements. All the array elements should have a value between 1 to n inclusive. Otherwise "No". The program aborts when it reaches the line at if(bSort). Is there anything wrong with the iterator increment?

#include <iostream>
#include <string>
#include <bits/stdc++.h>

using namespace std;

std::string solve(vector <int> &a, int n) {
    vector<int> visited (n);
    int i=0;
    for(std::vector<int>::iterator it = a.begin(); it != a.end(); ++it) {
        i++;
        if((it+1)!=a.end() && (*it > *(it+1)))
        {
           bSort = false;
        }
        if(std::find(visited.begin(), visited.end(), *it)!=visited.end())
        {
            return "No";
        }
        else
        {  
            visited[i] = *it;
        }

        if(*it <= 0 || *it > n)
        {
            return "No";
        }     

    }
    if(bSort)
        return "No";
    else
        return "Yes";

}

int main() {
    int q;
    cin >> q;
    for(int a0 = 0; a0 < q; a0++){
        int n;
        cin >> n;
        vector<int> a(n);
        for(int a_i = 0; a_i < n; a_i++){
           cin >> a[a_i];
        }
        std::string result = solve(a,n);
        cout << result << endl;
    }
    return 0;
}

The issue appears to be happening only with the following input:

1
30
18 8 24 20 7 17 5 9 26 21 25 12 11 15 30 13 19 16 22 10 14 1 3 29 23 2 6 28 4 27
satheesh v
  • 25
  • 2
  • 7
  • 1
    And if you use a debugger to catch the crash, what does it tell you? Where in your code does it happen? What are the values of all involved variables then? Where are the iterators pointing? – Some programmer dude Sep 07 '17 at 16:42
  • `#include ` -- Don't do this. Include the proper header files. – PaulMcKenzie Sep 07 '17 at 17:03
  • Good write up on [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h). But when it's used alongside `using namespace std;` not only does the code include the entire standard library, it pulls the entire standard library into the global namespace where tens of thousands of identifiers, some of them very common names like `swap` and `reverse`, may conflict with identifiers defined in your code for unexpected results. – user4581301 Sep 07 '17 at 17:13

1 Answers1

1

I'm not sure the problem has to do with iterators specifically.

At the very beginning of the loop, variable i is incremented before it's used, which means the set of numbers that i will range between is [1, vector.size()]. This means that at some point, you'll access vector[vector.size()], which is undefined behavior and can crash your program.

In your program specifically, given the input you provided, because none of the numbers in your example code are duplicated, the else branch of the std::find(...) conditional statement is always executed, which means you end up calling visited[30] at some point, which again, is out of bounds and undefined behavior, potentially causing a crash.

Xirema
  • 18,577
  • 4
  • 26
  • 60