Segmentation fault caused by addition operation

[SkaN]

Hello. I occurred a problem. Script faults with "Segmentation fault". GDB points command sum += arr[i]; in the Row::mean() function. All the variables are defined, I just didn't put them here. To download the whole program you should go to the link: http://ubuntuone.com/0vlfgW5kIEBTcWQV4FIYAq

Row::Row(int l)
{
    arr = new double[l];
    length = l;
}

double Row::mean()
{
    double sum = 0;

    for ( int i = 0; i < length; i++ )
    {
        sum += arr[i];
    }

    return ( sum / length );
}

Matrix::Matrix(int h, int w)
{
    matrix = new Row[h];
    for ( int i = 0; i < h; i++ )
    {
        matrix[i] = *(new Row(w));
    }
    width = w;
}

double Matrix::mean()
{
    double sum = 0;

    for ( int i = 0; i < width; i++ )
    {
        sum += matrix[i].mean();
    }

    return ( sum / width );
}

int main(int argc, char** argv)
{
    for ( int i = 0; i < h; i++ )
    {
        for ( int j = 0; j < w; j++ )
        {
            cout << "Type element [" << i << ", " << j << "]: ";
            cin >> matrix[i][j];
        }
    }
    
    meanBefore = matrix.mean();
}

[cire]

Matrix::Matrix(int h, int w)
{
    matrix = new Row[h];
    for ( int i = 0; i < h; i++ )
    {
        matrix[i] = *(new Row(w));
    }
    width = w;
}

Hello leaking memory. Depending on how Row::operator= is implemented, hello segmentation fault and memory corruption.

[SkaN]

I have similar program, that is working the same way and there's no Row::operator= implementation. If you want, I can upload it

[28poGNh0]

why not just post it here ,
and if you can provide the whole code source of the first one It will be helpful for some of us

[IWishIKnew]

You MUST use operators on only single elements.

Since your array is bi-demensional, you can not add an entire row by referencing only a single demension.

In my experience, segmentation faults occur when an element is called from an array, but that element doesn't exist. I have not worked with arrays enough to know if that kind of call would be caught by the compiler, but I suspect that it would for one reason: There is no + or = operator assigned to add rows.

You need to keep in mind that you can only use modifiers on single elements at a time, when you work with arrays, or any object (for that matter). You can write a function, though, to add rows up if you wish.

I would strongly suggest useing a class for this, because it appears you are trying to create functions to modify a single structure.

[ne555]

> there's no Row::operator= implementation
and that's exactly your problem. The one that the compiler provides would simply copy the pointers, so you'll have several objects that points to the same place, and try to delete it.

You may simply use an std::vector, or an std::valarray.

[SkaN]

#include <iostream>

using std::cin;
using std::cout;


class Row {
public:
    
    Row() {
        n_ = 0;
    }
    
    Row(int n) {
        n_ = n;
        arr_ = new double[n];
    }
   
    double operator[](int i) const{
        return arr_[i];
    }
    
    double &operator[](int i){
        return arr_[i];
    }
    
    int IndexOfMax() {
        int index = 0;
        double max = arr_[0];
        for(int i = 1; i < n_; ++i) {
            if(arr_[i] > max) {
                index = i;
                max = arr_[i];
            }
        }
        return index;
    }
    
    void ShiftToBegin(int index) {
        if(index >= n_) {
            return;
        }
        double temp = arr_[index];
        for(int i = index; i > 0; --i) {
            arr_[i] = arr_[i-1];
        }
        arr_[0] = temp;
    }
    void PrintRow() {
        for(int i = 0; i < n_; ++i) {
            cout << arr_[i] << ' ';
        }
        cout << '\n';
    }

   
    ~Row() {
        delete[] arr_;
    }
private:
    double n_;
    double *arr_;
};

class Matrix {
public:
    
    Matrix() {
        n_ = 0;
        m_ = 0;
    }
    
    Matrix(int n, int m) : n_(n), m_(m) {
        rows = new Row[n];
        for(int i = 0; i < n; ++i) {
            rows[i] = *(new Row(m));
        }
    }
    
    Row operator[](int i) const {
        return rows[i];
    }
    Row &operator[](int i) {
        return rows[i];
    }
    
    void MaxToBegin() {
        int indexOfMax;
        for(int i = 0; i < n_; ++i) {
            indexOfMax = rows[i].IndexOfMax();
            rows[i].ShiftToBegin(indexOfMax);
        }
    }
    bool FirstLowerThan(double border) {
        bool result = true;
        for(int i = 0; i < n_ && result; ++i) {

        }

        return result;
    }
    void PrintMatrix() {
        for(int i = 0; i < n_; ++i) {
            rows[i].PrintRow();
        }
    }



    ~Matrix() {
        delete[] rows;
    }
private:
    int n_;
    int m_;
    Row *rows;
};

int main() {
    int n,m;
    double border;
    int i,j;
    cout << "Enter n: ";
    cin >> n;
    cout << "Enter m: ";
    cin >> m;
    Matrix matrix(n,m);
    for(i = 0; i < n; ++i) {
        for(j = 0; j < m; ++j) {
            cout << "Enter element [" << i << "," << j << "]: ";
            cin >> matrix[i][j];
        }
    }
    cout << "Matrix:\n";
    matrix.PrintMatrix();
    matrix.MaxToBegin();
    cout << "Changed matrix:\n";
    matrix.PrintMatrix();
    cout << "Enter border: ";
    cin >> border;
    if(matrix.FirstLowerThan(border)) {
        cout << "Äà, è ïðàâäà ìåíüøå.";
    } else {
        cout << "Îëîëî, íå ñðàáîòàëî.";
    }

    return 0;
}

this is the program that works. It's built the same way as mine. My full program is availible by the link http://ubuntuone.com/0vlfgW5kIEBTcWQV4FIYAq . There are 5 files, so I think it's better to post it this way

[cire]

this is the program that works. It's built the same way as mine.

And it suffers from the same flaws. An incorrect program may work as you expect it to. That doesn't make it any less incorrect.

At a minimum, Row and Matrix should each implement a copy constructor, a copy assignment operator and a destructor in order to correctly manage the dynamic memory.

[SkaN]

a copy constructor, a copy assignment operator

Can you, please, give me an example of that operators? I'll be very thankful

and a destructor

Both classes have a destructor, if I understood you right

[Daleth]

struct Dummy{
   Dummy(const Dummy&); //Copy constructor
   Dummy& operator=(const Dummy&); //copy assignment operator
   ~Dummy(); //destructor
   char* cstring;
   size_t str_size;
   protected:
      void deep_copy(const Dummy&);
};

Dummy::Dummy(const Dummy& src)
   : cstring(nullptr)
   , str_size(0)
{deep_copy(src);}
Dummy& Dummy::operator=(const Dummy& src){
   if(this == &src) return *this;
   deep_copy(src);
   return *this;
}
Dummy::~Dummy(){
   delete[] cstring;
   cstring = nullptr;
}

void Dummy::deep_copy(const Dummy& src){
   delete[] cstring;
   cstring = new char[src.str_size];
   str_size = src.str_size;
//Or the appropiate, standard c-string functions
   for(size_t i(0); i < str_size; ++i)
      cstring[i] = src.cstring[i];
}

Edit:
Mistakes.

[ne555]

Beware of self-assignment

Also, if you say that you are going to return something, then return something.

[SkaN]

Thanks very much :)