Heap Corruption

[C++H]

Okay - warum ich std::string und std::vector etc. nicht benutze ist einfach zu erklären: Ich will gerade auf diesem Gebiet ohne zusätzliche Container arbeiten um u.A. mit solchen Problemen wie diesem Erfahrung zu gewinnen.

Eventuell ist mein = Operator das Problem (dort erhalte ich auch beim ersten delete[] den o.g. Error), den ich auch in meinem Kopierkonstruktor verwende:

BinaryInt& BinaryInt::operator=(const BinaryInt& bi)
{
	if (*this != bi)
	{
		delete [] cIntDec;
		delete [] cIntBin;

		cIntDec = new char[strlen(bi.cIntDec) + 1];
		cIntBin = new char[strlen(bi.cIntBin) + 1];

		strcpy(cIntDec, bi.cIntDec);
		strcpy(cIntBin, bi.cIntBin);
	}

	return *this;
}

[DocShoe]

1. Du vergleichst auf Gleichheit der Werte, nicht auf Gleichheit der Objekte.
2. wenn der zweite new[] Aufruf fehlschlägt erzeugt dein Code ein Speicherleck, da für den ersten new[] niemals delete[] aufgerufen wird. Eine Möglichkeit ist die Verwendung von new (nothrow)[] . Ausserdem sollte deine Klasse exception safe sein, d.h. ein Objekt sollte sich beim Auftreten einer Exception nicht verändern nach dem Auftreten einer Exception in einem definierten Zustand befinden. Wenn in Zeile 8 oder 9 eine bad_alloc Exception geworfen wird sind die member cIntDec und cIntBin schon zerstört und zeigen in´s Nirvana.

[Travor]

Ist absolut ungeprüft.. Aber in etwa so wie es laufen sollte:

template< typename T >
bool __inline SafeNewArray( T*& instance, size_t anzahl )
{
	try
	{
		instance = new T[ anzahl ];
	}
	catch( std::bad_alloc& /*not_needed*/)
	{
		instance = 0;
		return false;
	}
	return true;
}

template< typename T >
void SafeDeleteArray( T*& p )
{
	delete[] p;
	p = 0;
}

BinaryInt& BinaryInt::operator=( const BinaryInt& bi )
{
    //if (*this != bi)
	if ( this != &bi )	// Kein versteckter Operator==(... )
    {
        //delete [] cIntDec;
        //delete [] cIntBin;

		size_t newSizeDec = strlen( bi.cIntDec ) + 1;
		size_t newSizeBin = strlen( bi.cIntBin ) + 1;
		if ( newSizeDec != 0  &&  newSizeBin != 0 )
		{
			char* decTmp = 0;
			char* binTmp = 0;
			if ( !SafeNewArray( decTmp, newSizeDec )  ||  !SafeNewArray( binTmp, newSizeBin ) )
			{
				// Zu wenig Speicher...
				// Was ist zu tun ?

				// z.B. alles lassen wie es ist.
				SafeDeleteArray( decTmp );
				SafeDeleteArray( binTmp );
				return ( *this );
			}

			//strcpy(cIntDec, bi.cIntDec);
			//strcpy(cIntBin, bi.cIntBin);

			// Die strings sicher kopieren...
			// Hie stehen Dir je nach OS/Compiler unterschiedliche Möglichkeiten zu Verfügung.
			errno_t result = strcpy_s( decTmp, newSizeDec, bi.cIntDec );
			if ( result != 0 )
			{
				// Fehler- blabla...
				// Was ist zu tun ?
			}

			result = strcpy_s( binTmp, newSizeBin, bi.cIntBin );
			if ( result != 0 )
			{
				// Fehler- blabla...
				// Was ist zu tun ?
			}

			// Alles richtig.
			SafeDeleteArray( cIntDec );
			SafeDeleteArray( cIntBin );

			cIntDec = decTmp;
			cIntBin = binTmp;
		}
		else
		{
			// Overflow -> was ist zu tun ?
			// Ist aber unrealistisch, da die Konstruktoren vorher schon aussteigen würden.
		}

    }

    return *this;
}

[SeppJ]

Warum verändert dein SafeDeleteArray den Zeigerwert? Willst du mögliche Fehler vertuschen, anstatt es schon beim Debuglauf krachen zu lassen?

Was soll das überhaupt? Deine SafeXXXArrays sind vielleicht gegen Exceptions sicher die sowieso nie fliegen, aber nicht gegen ganz normale Exceptions im Code oder nachlässige Programmierer. Ein RAII-Design wäre sicher gegen alles

[DocShoe]

Mal ganz ehrlich... wenn man soweit ist, solchen Code zu verwenden, dann kann man sich gratulieren und zugestehen, dass man die Problematik verstanden hat. Anschließend wirft man das raus und benutzt std::string/std::vector .

[Travor]

DocShoe schrieb:

Mal ganz ehrlich... wenn man soweit ist, solchen Code zu verwenden, dann kann man sich gratulieren und zugestehen, dass man die Problematik verstanden hat. Anschließend wirft man das raus und benutzt std::string/std::vector .

richtig
aber es wurde ja ausdrücklich so gewünscht:)

[Gelangweilter]

Würde es nicht einfach reichen, einen (unsigned) Integer zu warappen (kann ja auch 64 Bit breit sein). Dann würden sich nämlich viele Dinge vereinfachen. Vor allem das Hinzufügen von neuen Operatoren (|, ^ etc.) würde sich vereinfachen:

class BinInt
{
	typedef unsigned bi_type;
public:
	BinInt() : val_(0) {}
	BinInt(const BinInt& b) : val_(b.val_) {}
	BinInt(bi_type v) : val_(v) {}
	BinInt operator &(const BinInt& b)
	{
		return BinInt(val_ & b.val_);
	}
	BinInt& operator &=(const BinInt& b)
	{
		val_ &= b.val_;
		return *this;
	}
	BinInt& operator =(const BinInt& b)
	{
		val_ = b.val_;
		return *this;
	}
	BinInt operator &(bi_type b)
	{
		return BinInt(val_ & b);
	}
	BinInt& operator &=(bi_type b)
	{
		val_ &= b;
		return *this;
	}

	friend ostream& operator<<(ostream& o, const BinInt& b);

private:
	bi_type val_;
};

ostream& operator <<(ostream& o, const BinInt& b)
{
	for(int i=sizeof(BinInt::bi_type)*CHAR_BIT-1;i>=0;--i)
	{
		o<<((b.val_>>i) & 1?'1':'0');
	}
	return o;
}

[C++H]

@Travor:
Ich habe mal deinen Code umgesetzt. Jetzt crasht es, aber schon beim strcyp_s ("Zugriffsverletzung").

@DocShoe:
Ich weiß, dass man sowas in der Praxis nicht machen wird - zumal std::string und std::vector perfekt funktionieren. Mir geht es, aber (wie schon erwähnt) um den Lerneffekt.

[SeppJ]

Der objektorientierte Designer sagt: Wieso soll sich deine BinaryInt Klasse um Implementierungsdetails ihres Datenspeichers kümmern müssen? Schreib eine Klasse BinaryString, die kümmert sich um ihren eigenen Speicherplatz, weiß wie sie selber kopiert wird, weiß selber wie man ihre Länge misst, usw.

Diese Klasse wird dann von deinem BinaryInt benutzt, der BinaryInt kümmert sich dann nur noch um seine eigenen Sachen und nutzt deren zur Verügung gestellte Funktionalität, um sein Ding zu machen.

Das nennt man dann Kapselung.

[Travor]

C++H schrieb:

@Travor:
Ich habe mal deinen Code umgesetzt. Jetzt crasht es, aber schon beim strcyp_s ("Zugriffsverletzung").

Ich befürchte Du muss dein Code hier komplett stellen damit wir den Fehler entdecken können.