0

Buenas, tengo este programa de ejemplo y esta clase, donde al ejecutarla libero memoria dos veces y no entiendo el por qué.

Esta es la clase date que se encarga de leer y gestionar fechas y eventos históricos. El operador >> lee ficheros de la forma AÑO#EVENTO1#EVENTO2#...#EVENTON. Ejemplo:

1908#Fantasmagorie#The Taming of the Shrew#The Thieving Hand#The Assassination of the Duke of Guise#A Visit to the Seaside

Este es el código:

#include<iostream>
#include<string>
#include<fstream>
#include<sstream>
#include<date.h>

using namespace std;

void date::resize(int r)
{
    assert(r>=0);
    if(r!=this->reserved)
    {
     if(r!=0)
     {
        string * aux = new string[r];
        if(this->reserved>0)
        {
            int min=this->reserved<r?this->reserved:r;
            for(int i=0; i<min; i++)
                aux[i]=this->str[i];
            delete[] this->str;
        }
        this->str=aux;
        this->reserved=r;
        if(this->reserved<this->eventsNum)
            this->eventsNum=this->reserved;
     } else
     {
        if(this->reserved>0)
            delete[] this->str;
        this->year=0;
        this->eventsNum=0;
        this->reserved=0;
     }
    }
}

date::date() : year(0), eventsNum(0), reserved(0), str(0){}

date::date(const date& d)
{
    this->year=d.year;
    this->eventsNum=d.eventsNum;
    this->reserved=d.reserved;
    this->str=new string[this->reserved];
    for(int i=0; i<this->eventsNum; i++)
        this->str[i]=d.str[i];
}

date::~date()
{
    this->year=0;
    this->eventsNum=0;
    this->reserved=0;
    if(this->str)
    delete[] this->str;
    this->str=NULL;
}

int date::getAge(){return this->year;}

ostream& operator<<(ostream& os, const date& d)
{
    os << d.year;
    for(int i=0; i<d.eventsNum; i++)
        os << '#' << d.str[i];
    os << endl;
    return os;
}

istream& operator>>(istream& is, date& d)
{
    string line; char c;
    is >> d.year >> c;
    getline(is, line);

    int n=1;
    for(int i=0; i<line.length(); i++)
        if(line[i]=='#')
            n++;

    d.eventsNum=n;
    d.reserved=d.eventsNum;
    delete[] d.str;
    d.str=new string[n];

    stringstream ss(line);

    for(int i=0; i<n; i++)
        getline(ss, d.str[i], '#');

    return is;
}

Y este es el programa que tengo para probar el buen funcionamiento de la clase:

#include<iostream>
#include<fstream>
#include<cronologia.h>
#include<date.h>

using namespace std;

int main(int argc, char * argv[]){
    cout <<  "STATE: IN PROGRESS" << endl;
    cout << "TEST: (2)" << endl;
    date d;

    ifstream f("./data/name.txt");

    while(f >> d)
    {
        cout << d;
    }
    date d1;
    cin >> d1;
    d=d1;
    cout << d << endl;


}

El error me salta cuando le paso por teclado lo siguiente:

1908#Fantasmagorie#The Taming of the Shrew#The Thieving Hand#The Assassination of the Duke of Guise#A Visit to the Seaside

... o cualquier frase en ese formato.

El error es el siguiente:

Error in `./bin/test': double free or corruption (fasttop): 0x00000000019c07c0

Llevo mucho rato mirando a ver como solucionar lo de liberar dos veces la memoria pero no veo forma. ¿Alguna solución?

Dejo algunas frases para que probéis la clase si os hace falta.

1900#Sherlock Holmes Baffled#The Enchanted Drawing
1901#Star Theatre#Scrooge, or, Marley's Ghost
1902#A Trip to the Moon
1903#The Great Train Robbery#Life of an American Fireman
1904#The Impossible Voyage
1905#Adventures of Sherlock Holmes; or, Held for Ransom
1906#The Story of the Kelly Gang#Humorous Phases of Funny Faces#Dream of a Rarebit Fiend
1907#Ben Hur#L'Enfant prodigue
1908#Fantasmagorie#The Taming of the Shrew#The Thieving Hand#The Assassination of the Duke of Guise#A Visit to the Seaside
1
  • 1
    Se te ha olvidado indicar la cabecera de date
    – eferion
    el 28 oct. 2017 a las 22:38

1 respuesta 1

1

A falta de que publiques la cabecera de date yo observo un comportamiento peligroso aquí:

d=d1;

¿Por qué?

En el fichero de implementación no aparece el operador de asignación. Dado que usas memoria dinámica, si no implementas manualmente este operador lo que sucede es que el compilador puede acabar creando su propia versión (que se limita a copiar los miembros de una instancia a otra)... que vendría a ser algo tal que:

date& date::operator=(date const& d)
{
  year=d.year;
  eventsNum=d.eventsNum;
  reserved=d.reserved;
  str=d.str; // <---

  return *this;
}

Merece especial atención la línea comentada por dos motivos:

  • No libera la memoria que hubiese reservada anteriormente, lo que genera fugas de memoria
  • Hace que dos instancias diferentes apunten a la misma reserva de memoria... lo que puede provocar fácilmente dobles borrados.

Para solucionar este error basta con crear una versión propia del operador de asignación que lidie correctamente con la memoria dinámica. Una posible implementación:

date& date::operator=(date const& d)
{
  year=d.year;
  eventsNum=d.eventsNum;
  reserved=d.reserved;

  delete[] str;
  str = new string[reserved];

  for( int i=0; i<eventsNum; i++)
    str[i] = d.str[i];

  return *this;
}

Otra solución bastante tonta pasa por omitir la copia de las fechas:

int main(int argc, char * argv[]){
    cout <<  "STATE: IN PROGRESS" << endl;
    cout << "TEST: (2)" << endl;
    date d;

    ifstream f("./data/name.txt");

    while(f >> d)
    {
        cout << d;
    }
    cin >> d;
    cout << d << endl;
}

El problema que le veo a esta solución es que tu clase seguiría siendo insegura de cara a futuros usos.

Nota final Nota que no hace falta usar this de forma sistemática para acceder a los miembros de una clase.

Tu Respuesta

Al pulsar en “Publica tu respuesta”, muestras tu consentimiento a nuestros términos de servicio, política de privacidad y política de cookies

¿No es la respuesta que buscas? Examina otras preguntas con la etiqueta o formula tu propia pregunta.