0

According to other suggestion dereferencing may be issue but I am getting segmentation fault even before derefencing while calling the max_element function.

Minimal reproducible example :

#include <bits/stdc++.h>
using namespace std;

class B
{
    public:
    int a, b;
    int getA()
    {
        return a;
    }
    int getB()
    {
        return b;
    }
};
class A
{
    public:
    vector<B> array;
    A()
    {
        array.resize(5);
        array[0].a = array[0].b = 3;
        array[1].a = array[1].b = 5;
        array[2].a = array[2].b = 7;
        array[3].a = array[3].b = 1;
        array[4].a = array[4].b = 2;
    }
    vector<B> getArray()
    {
        return array;
    }
};

int main()
{
    A aobj;
    B maxE = *std::max_element(aobj.getArray().begin(), aobj.getArray().end(),
        [](B jobA, B jobB) {
        return jobA.getA() < jobB.getA();
    });
    
    cout<<maxE.getA();
    return 0;
}
  • should be `std::vector& getArray(){ return array; }` – Jarod42 Mar 16 '21 at 14:28
  • https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h – akira hinoshiro Mar 16 '21 at 14:29
  • read about **reference**. – Jarod42 Mar 16 '21 at 14:30
  • But why was I getting Segmentation fault –  Mar 16 '21 at 14:39
  • @akirahinoshiro in my production env I wasn't using bits but just to quickly reproduce this issue I have used bits otherwise I know the fact "why bits/stdc++ should not be used" –  Mar 16 '21 at 14:41
  • 1
    You access destroyed temporary element, and even if lifetime was extended, you use 2 different container for the limit, so you will have out-of-bound access. – Jarod42 Mar 16 '21 at 14:41
  • I have a different reason for not using bits/stdc++: Including the entire C++ Standard library increases the build time of a small program by a factor of close to 10. Whatever time you saved in providing a single header will be eaten up in a few builds of the test case. – user4581301 Mar 16 '21 at 15:15

1 Answers1

2

The member function of A

    vector<B> getArray()

is returning a copy of the member variable array. Calling it two times will generate two independent copies, so using them with std::max_element is dangerous.

You should have the function return a reference to the member variable array.

    vector<B>& getArray() // add &
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
  • Or `const vector& getArray()`. – 1201ProgramAlarm Mar 16 '21 at 14:34
  • But what's the significance of segmentation fault here? –  Mar 16 '21 at 14:35
  • @FarhanAhmed -- in the code in the question, `getArray().begin()` and `getArray().end()` are iterators into two different arrays. That means that they do not delineate a valid range, and when they are passed to an algorithm that assumes they do, bad things can happen. – Pete Becker Mar 16 '21 at 14:48
  • @FarhanAhmed the two iterators must be to the same container if not you have UB. Probably the algorithm tries to reach the end of one array by incrementing the begin of another array and accesses memory that isnt part of either of them – 463035818_is_not_an_ai Mar 16 '21 at 14:48