Hello. Faced the problem of double calling the destructor (the destructor is called 2 times in a row for 1 and the same object).

Here is the calling code in main ():

CharRow row(4); CharRow row2(3); cin >> row; cin >> row2; CharRow temp = row - row2;

CharRow is a class that contains a pointer to a string.

The problem occurs in the overloaded operator- ():

  CharRow CharRow::operator- (CharRow& row2) { int row1Length = this->getRowCurrentLength(); int row2Length = row2.getRowCurrentLength(); CharRow tempRow(row1Length - row2Length); char source[row1Length]; for (int i = 0; i < row1Length; i++) { source[i] = this->ptr[i]; } char* tmpStr; tmpStr = strstr(source, row2.ptr); //if row2 is not substring for row1 if (tmpStr == NULL) { return *this; } strcpy(tmpStr, tmpStr + row2Length); for (int i = 0; i < row1Length - row2Length; i++) { tempRow.ptr[i] = source[i]; } return tempRow; }

after line

 return tempRow;

The copy constructor is called:

 CharRow::CharRow(const CharRow &obj){ this->ROW_MAX_LENGTH = obj.ROW_MAX_LENGTH; this->ptr = obj.ptr; }

After the copy constructor, the destructor is called:

 CharRow::~CharRow(){ //ptr = NULL; delete [] ptr; }

And then this destructor is called again and the program crashes (because the destructor tries to delete the null pointer (as I understand it).

Help to reach a complete understanding of the problem and suggest how to get out of this situation correctly.

    2 answers 2

    In this sentence

     CharRow temp = row - row2;

    a temporary object is created. First, this temporary object may contain a copy of the ptr pointer of the row object.

     CharRow CharRow::operator- (CharRow& row2) { // ... //if row2 is not substring for row1 if (tmpStr == NULL) { return *this; } //... }

    Secondly, a copy of the same pointer is assigned to the created temp object. And all this is due to the fact that your copy constructor simply assigns the value of a given pointer from one object to another object.

     CharRow::CharRow(const CharRow &obj){ this->ROW_MAX_LENGTH = obj.ROW_MAX_LENGTH; this->ptr = obj.ptr; }

    So even the situation when the same memory will be deleted not only twice, but also three times is not excluded! And all because all three objects contain pointers with the same value that indicate the same memory area.

    When you manually allocate memory, you need to do a "deep copy", that is, not just assign one pointer to another, but create a new memory area and copy the data there so that each pointer points to its own allocated memory.

    Therefore, you need to explicitly define at least the copy constructor, the copy assignment operator, and the destructor.

    Keep in mind that the C ++ standard does not support variable-length arrays, and therefore declarations like this

     int row1Length = this->getRowCurrentLength(); // ... char source[row1Length];

    are not compatible with the C ++ standard.

    In addition, the operator operator - you probably have an undefined behavior, since you use functions for working with strings, and the declared array most likely does not contain a terminating zero.

    • And you can advice how you can replace int row1Length = this->getRowCurrentLength(); - ReturnedVoid
    • @ReturnedVoid And why should you replace? Just do not need to use this array. - Vlad from Moscow
    • I use this array as temporary and I need to somehow find out its size ... - ReturnedVoid
    • Changed the CopyConstructor to this code: CharRow::CharRow(const CharRow &obj){ this->ROW_MAX_LENGTH = obj.ROW_MAX_LENGTH; ptr = new char[ROW_MAX_LENGTH]; for(int i = 0; i < ROW_MAX_LENGTH; i++){ this->ptr[i] = obj.ptr[i]; } } CharRow::CharRow(const CharRow &obj){ this->ROW_MAX_LENGTH = obj.ROW_MAX_LENGTH; ptr = new char[ROW_MAX_LENGTH]; for(int i = 0; i < ROW_MAX_LENGTH; i++){ this->ptr[i] = obj.ptr[i]; } } CharRow::CharRow(const CharRow &obj){ this->ROW_MAX_LENGTH = obj.ROW_MAX_LENGTH; ptr = new char[ROW_MAX_LENGTH]; for(int i = 0; i < ROW_MAX_LENGTH; i++){ this->ptr[i] = obj.ptr[i]; } } The problem disappeared, but nevertheless, in 1 situation, it still arises ( - ReturnedVoid
    • @ReturnedVoid What is 1 situation? You still need to write a copy assignment operator. - Vlad from Moscow

    Your problem is here:

     CharRow::CharRow(const CharRow &obj){ this->ROW_MAX_LENGTH = obj.ROW_MAX_LENGTH; this->ptr = obj.ptr; }

    Apparently ptr points to a dynamically allocated memory (otherwise you would not resort to delete :)). You copy the pointer , not the contents of the memory, so you have two objects that have the same pointer to the same place in memory. Which you as a result at least delete twice, or, for example, having deleted once, try to work with it.

    Copy content - select a new memory and copy there the data from the memory pointed to by another object.

    PS By the way, delete null pointer is a completely allowed operation, delete checks for a pointer to equality to zero.