Crash pamięci przy zwalnianiu zaalkowanej pamięci

Question

Cześć,

Pracuje na VS Enterprise 2017. Przy zwalnianiu zaalkowanej pamięci VS wywala błąd: Critical error detected c0000374.

Czy ktoś się spotkał z podobnym zachowaniem, bardzo proszę o pomoc.

class Stock
{
private:
    char * company;
    int shares;
    double share_val;
    double total_val;
    void set_tot() { total_val = shares * share_val;}
public:
    Stock();
    Stock(const char * co, long n = 0, double pr = 0.0);
    ~Stock();

}

Stock::Stock()
{
    //company = new char[1];
    company = nullptr;
    ile_obiektow++;
};

Stock::Stock(const char *co, long n, double pr)
{
    int ile = std::strlen(co);
    company = new char[ile+1];
    strcpy_s(company,KOT,co);
    ile_obiektow++;
};

Stock::~Stock()
{
    delete[] company;
    ile_obiektow--;
};

int main()
{
    {
        Stock firma1;
        Stock firma2 = "sdasdas";
        Stock firma3 = firma2;
    }
    return 0;
}

 

Answer 1

Poczytaj o rule of five. W skrócie: jeśli klasa alokuje wewnętrznie jakąś pamięć, to powinieneś dla niej zdefiniować:

• destructor
• copy ctor
• copy assignment operator
• move ctor
• move assignment operator

https://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11 

Answer 2

Zrób operator przypisania i konstruktor kopiujący, bo teraz są domyślne, co powoduje, że pamięć spod company jest usuwana dwa razy przez oryginał (firma2) i kopię obiektu (firma3).

 

PS. kod wstawiaj w {...}

Comments

Dzięki za wsparcie. Dodałem konstruktor kopiujący, operator przypisania, konstruktor przenoszący ale teraz mam crash przy alokowaniu pamięci dla nowego obiektu firma3(konstruktor kopiujący)- nie mam pojęcia dlaczego. Będę bardzo wdzięczny za jakiekolwiek uwagi...

Pomijając w mainie tworzenie obiektu firma3, przy zwalnianiu pamięci dalej dostaję crasha. Czy macie pomysł dlaczego?

PS.Jeżeli odpalam kod w Code block'sie wszystko jest OK.

class Stock
{
private:
    char * company;
    int shares;
    double share_val;
    double total_val;
    void set_tot() { total_val = shares * share_val;}
public:
    Stock();
    Stock(const char * co, long n = 0, double pr = 0.0);
    ~Stock();
	Stock(const Stock & s);
	Stock & operator=(const Stock & s);
        Stock(Stock &&);
}
 
Stock::Stock()
{
    //company = new char[1];
    company = nullptr;
    ile_obiektow++;
};
 
Stock::Stock(const char *co, long n, double pr)
{
    int ile = std::strlen(co);
    company = new char[ile+1];
    strcpy_s(company,KOT,co);
    ile_obiektow++;
};
 
Stock::~Stock()
{
    delete[] company; //przy pominięciu tworzenia obiektu firma3 w tym miejscu crash heapa
    ile_obiektow--;
};
 
Stock::Stock(const Stock & s)
{
	int ile = std::strlen(s.company);
	delete[]company;
	company = new char[ile + 1];//w tym miejscu CRASH
	strcpy_s(company, KOT, s.company);
	shares = s.shares;
	share_val = s.share_val;
};

Stock & Stock::operator=(const Stock & s)
{
	if (this == &s) return *this;

	int ile = std::strlen(s.company);
	delete[]company;
	company = new char[ile + 1];
	strcpy_s(company, KOT, s.company);
	shares = s.shares;
	share_val = s.share_val;
	return *this;
};

Stock::Stock(Stock && s)
{

};

int main()
{
    {
        Stock firma1;
        Stock firma2 = "sdasdas";
        Stock firma3 = firma2;
    }
    return 0;
}

 

---

Co to jest KOT? Dlaczego używasz strcpy_s, skoro miejsce docelowe zawsze będzie miało wystarczająco dużo miejsca?

 

Zaimplementuj ctor przenoszący, bo teraz nic nie robi, nawet nie zeruje wskaźnika company.

Jak już masz ctor przenoszący, to powinieneś też zrobić operator przenoszący (rule of five).

---

Nie rób delete[] company;. To nie ma żadnego sensu, bo to konstruktor. Więc wskaźnik company jest niezainicjalizowany, tj. możliwe że ma jakąś niezerową wartość a to oznacza że delete [] próbuje coś zwolnić podczas gdy to z pewnością nie jest adres zablokowanej pamieci. Po prostu pozbądź się tej dealokacji i copy ctor będzie ok. W copy assignment dealokacje oczywiście zostaw bo tam ma sens.

Jak pisałem w swojej odpowiedzi, move ctor I move assignment tez powinny być zaimplementowane, ale w obecnym kodzie nie są używane.

---

Uzupełniłem kod, ale nie rozumiem  w jakim celu w tym przypadku potrzebuję ctor przenoszący i operatator przenoszący skoro nie ma żadnych r-wartości. Dodałem ww. ctor i operator ale puste, przy debugowaniu kodu kompilator nawet tam nie zajrzał. Kod dalej się crashuje przy cons kopiującym (firma3 moment rezerwacji pamięci). Macie pomysł dlaczego?

const int KOT = 55;
static int ile_obiektow = 0;

class Stock
{
private:
    char * company;
    int shares;
    double share_val;
    double total_val;
    void set_tot() { total_val = shares * share_val;}
public:
    Stock();
    Stock(const char * co, long n = 0, double pr = 0.0);
    ~Stock();
    Stock(const Stock & s);
    Stock & operator=(const Stock & s);
    Stock(Stock &&);
    Stock & operator=(Stock && s);
}
  
Stock::Stock()
{
    //company = new char[1];
    company = nullptr;
    ile_obiektow++;
};
  
Stock::Stock(const char *co, long n, double pr)
{
    int ile = std::strlen(co);
    company = new char[ile+1];
    strcpy_s(company,KOT,co);
    ile_obiektow++;
};
  
Stock::~Stock()
{
    delete[] company; //przy pominięciu tworzenia obiektu firma3 w tym miejscu crash heapa
    ile_obiektow--;
};
  
Stock::Stock(const Stock & s)
{
    int ile = std::strlen(s.company);
    //delete[]company;
    company = new char[ile + 1];//w tym miejscu CRASH
    strcpy_s(company, KOT, s.company); //KOT TO STAŁA int = 55
    shares = s.shares;
    share_val = s.share_val;
};
 
Stock & Stock::operator=(const Stock & s)
{
    if (this == &s) return *this;
 
    int ile = std::strlen(s.company);
    delete[]company;
    company = new char[ile + 1];
    strcpy_s(company, KOT, s.company);
    shares = s.shares;
    share_val = s.share_val;
    return *this;
};
 
Stock::Stock(Stock && s)
{
 
};

Stock & Stock::operator=(Stock && s)
{
	return *this;
};
 
int main()
{
    {
        Stock firma1;
        Stock firma2 = "sdasdas";
        Stock firma3 = firma2;
    }
    return 0;
}

 

---

Funkcje takie jak strlen czy strcpy wyłożą się, gdy dostaną wskaźnik równy nullptr.

Stock::Stock(const Stock & s)
    :company(nullptr)
{
    if(s.company)
    {
        int ile = std::strlen(s.company);
        company = new char[ile + 1];
        strcpy_s(company, KOT, s.company); 
    }
    
    shares = s.shares;
    share_val = s.share_val;
}
  
Stock & Stock::operator=(const Stock & s)
{
    if (this == &s) return *this;
  
    delete[]company;
    company = nullptr;
    
    if(s.company)
    {
        int ile = std::strlen(s.company);
        company = new char[ile + 1];
        strcpy_s(company, KOT, s.company);
    }
    
    shares = s.shares;
    share_val = s.share_val;
    return *this;
}

 

//KOT TO STAŁA int = 55

Po co ograniczasz długość łańcucha do 55 znaków, skoro używasz new?

---

Jeżeli używam strcpy to kompilator się buntuje i sugeruje użycie strcp_s jak wyżej. Strcpy_s wymaga podania maksymalnego rozmiaru bufora --> stała KOT.

Rozumiem, że przy podaniu wskaźnika nullptr jako parametr do funkcji będzie crash ale w tym przypadku program nawet nie dochodzi do strcpy_s... crashuje się przy rezerwacji pamięci dla obiektu firma2.

---

Strcpy_s wymaga podania maksymalnego rozmiaru bufora --> stała KOT.

Ale u Ciebie nie ma czegoś takiego jak maksymalna wielkość bufora, bo on zawsze będzie miał tyle, żeby pomieścić tekst źródłowy. Zamiast KOT użyj ile + 1.

 

kompilator się buntuje

link

 

ale w tym przypadku program nawet nie dochodzi do strcpy_s

U mnie ten kod (z poprawkami) kompiluje się (g++) i działa bez problemu.

---

Działa- moja bardzo wielka wina. Dzięki za pomoc