Sobrecarga operador+ y miembro puntero...

Iniciado por digimikeh, 18 Junio 2019, 22:11 PM

0 Miembros y 1 Visitante están viendo este tema.

digimikeh

Hola nuevamente:

El siguiente código:

Código (cpp) [Seleccionar]

#include <iostream>

using namespace std;

struct s{
   
private:
    double * d = nullptr;       
   
public:
    s() : d{new double}{}
    ~s(){ if (d != nullptr) delete d; }
    void setd(const double _d){ *this->d = _d; }
    double getd() const { return *this->d; }
   
};

s operator+(const s & _s0, const s & _s1){
    s ns;
    ns.setd(_s0.getd() + _s1.getd());
    return ns;
}

int main(){
   
    s s0;
    s s1;
   
    s s2;
    s2 = s0 + s1;
   
    return 0;
}



Aqui listo para ejecutar:
https://onlinegdb.com/rJNPQ68kr

Al ejecutar, obtengo el mensaje de error:

*** Error in `./a.out': double free or corruption (fasttop): 0x0000000000795c80 ***                                             
Aborted (core dumped)


Estoy seguro que el problema esta entre el puntero privado y el delete en el destructor, ya que si no uso puntero, compila bien.

Sin embargo, he puesto puntero porque quiero entender qué está pasando.  Me da la impresión de que en la definición del operador+ como se crea el objeto ns al terminar el ámbito, se está destruyendo y esta eliminando el puntero.  Lo cual parece valido, pero no tendría por qué corromper la ejecución, se supone que estoy destruyendo un objeto de copia, no los operandos utilizados.

Dungeons & dragons;
dragons.Attack();

RayR

#1
Lo que sucede es que cuando se llega aquí:

Código (cpp) [Seleccionar]
   s2 = s0 + s1;

el objeto temporal creado por el operador sobrecargado (ns) es asignado a s2. Esto hace que se ejecute el operador de asignación (operator=) para struct s. Dado que tú no programaste ninguno, se usa el generado por el compilador. Éste simplemente copia los valores de las variables miembro, es decir, que luego de la copia, s2.d apunta a la misma dirección que ns.d. Esa memoria se libera cuando ns es destruida. Y luego, cuando s2 es destruida, naturalmente, se vuelve a intentar liberar la misma dirección, lo cual es incorrecto. Que se trate de copias es irrelevante. Lo que importa es que apuntan a la misma dirección, y es un error intentar liberar memoria ya liberada. Pero eso ni siquiera es lo peor. Como te mencioné, en la línea 30, s2.d apuntará a la dirección de ns.d, pero antes de esa línea apuntaba a otra dirección distinta, la cual jamás fue liberada, por lo que tienes una "fuga de memoria" o memory leak.

La solución es sobrecargar también operator=, y en él no copiar los punteros, sino los valores de las direcciones a las que apuntan (reservando y liberando memoria, según sea necesario). Imagino que aún no habrás visto ese tema, pero ya llegarás y verás cómo implementarlo. En general, siempre que tengamos punteros habría que sobrecargar el operador de asignación y proporcionar un constructor de copia.

digimikeh

Tienes razón!... me había preocupado de sobrecargar el operador + y no pensé en las consecuencias, debí haber cargado ademas el operador = y después << para imprimirlo...

La lección que me dio esto es que debo pensar en cada sobrecarga de operador necesaria pues una lleva a la otra...

Gracias
Dungeons & dragons;
dragons.Attack();

Loretz


El que se mete con uno se está metiendo con todos...

https://en.cppreference.com/w/cpp/language/rule_of_three

Además:
Un comentario y un error:

El comentario es que cuando dices:
double getd() const { return *this->d; }
no necesitas aclarar que se trata del miembro 'd' de 'this', de eso no hay dudas. El puntero this se usa en las expresiones cuando es *realmente* necesario, por ejemplo, para salvar alguna ambigüedad (necesario), o porque los nombres de los miembros y parámetros ya han hecho un lío tal que no se sabe quién es quién (un parche que suele usarse para dejar en claro situaciones como "el lío este yo no lo hice así que ahora aguantarse el this").

Y el error:
getd() devuelve *d, y el problema aquí es que *d no fue inicializado, no se le asignó un valor; y pretender acceder en modo lectura (cuando haces s2 = s0 + s1;) a una variable sin inicializar es lo que el estándar define como "Undefined Behavior", que significa que puede suceder cualquier cosa (normalmente mala).




@XSStringManolo

Al hacer s2 = s0 + s1 que se supone que hace? Suma s0 d + s1 d ?

digimikeh

@#Loretz.. efectivamente, olvide darle un valor por default en el constructor.
Respecto al puntero this, pues si, es que el código es borrador, lo hago nada mas que para fines de practica... en un reléase, el this lo utilizo siempre y cuanod tenga dos o mas variables que tengan el mismo nombre..

Aproposito, muchas gracias por compartir lo de la regla de 3...
creo que defintivamente tengo que echarle una repasadita al constructor de copia ademas..


#@String Manolo, pues si, solo que como el puntero d es privado, estoy accediendo con el getter..

Dungeons & dragons;
dragons.Attack();