Heap Corruption

wieso nicht einfach:

for (int i=0; i < minsize; ++i)
{
 cIntBin[i] &= bi.cIntBin[i]
}

?
und die ganzen new/delete usw. sind weg...
Vorausgesetzt man hat hat in der Konstruktoren und Zuweisungen alles richtig gemacht.

P.S. Allerdings hast Du so nach wie vor einen Bug wenn Du bei der unterschiedlichen Längen... Die char-arrays müssen dann von links gefüllt werden.

daddy_felix

und wieso hast du ein char-Array? Was spricht gegen std::string/std::vector/std:bitset?

cIntBin = new char[strlen(res)];
    strcpy(cIntBin, res);

Fällt die da was auf? Mit std::string wäre es nicht passiert ...

C++H

@Travor:
Habe ich gemacht:

BinaryInt BinaryInt::operator&(const BinaryInt bi)
{
	int size	= MAX(strlen(cIntBin), strlen(bi.cIntBin)) + 1;		// + 1: Null Terminator
	int minsize = MIN(strlen(cIntBin), strlen(bi.cIntBin));

	//char* res = new char[size];

	for (int i=0; i < minsize; ++i)
		cIntBin[i] &= bi.cIntBin[i];
		//res[i] = (cIntBin[i] == '1' && bi.cIntBin[i] == '1') ? '1' : '0';

	//res[minsize] = '\0';

	//BinaryInt Copy(*this);
	//delete [] Copy.cIntBin;
	//Copy.cIntBin = new char[strlen(res)];
	//strcpy(Copy.cIntBin, res);
	//delete [] res;

	return *this;        // HEAP CORRUPTION #138
}

Jetzt tritt eine Heap Curruption #138 auf. Falls es an den Konstruktoren liegt, hier nochmal der Code:

BinaryInt::BinaryInt()
	: cIntBin(0), cIntDec(0)
{}

BinaryInt::BinaryInt(char* DecInt)
	: cIntBin(new char[strlen(DecInt)]), cIntDec(new char[5])
{
	strcpy(cIntBin, DecInt);
	strcpy(cIntDec, "55555");
}

BinaryInt::BinaryInt(const BinaryInt& bi)
	: cIntBin(new char[strlen(bi.cIntBin)]), cIntDec(new char[strlen(cIntDec)])
{
	strcpy(cIntBin, bi.cIntBin);
	strcpy(cIntDec, bi.cIntDec);
}

LordJaxom

Für einen String mit n Zeichen Länge brauchst Du n+1 Zeichen Platz. Aber eigentlich weißt Du das schon (oder hast Du den u.a. Kommentar kopiert?). Ergo: Du hast

// + 1: Null Terminator

mindestens fünfmal vergessen.

cIntBin(new char[strlen(DecInt)]

siehe oben

dot

Deine Arrays sind zu klein. Warum verwendest du nicht einfach std::string?
Und nimm doch std::min() bzw. std::max() oder verwend zumindest inline Funktionen und keine Makros dafür...

@C++H

Man hat Dir schon mehrmals beantwortet .
Und wie gesagt. So eine UND-operation funktioniert nur bei BigEndians richtig..
Da muss Du dir ganz bewusst vor Augen führen, was das für die Konsequenzen hat.

C++H

@LordJaxom:
Danke - irgendwie habe ich das übersehen. Nun bekomme ich eine "Debug Assertion Failed!": _BLOCK_TYPE_IS_VALID.
(Der Kommentar ist überings nicht kopiert)

BinaryInt BinaryInt::operator&(const BinaryInt bi)
{
	int size	= MAX(strlen(cIntBin), strlen(bi.cIntBin)) + 1;		// + 1: Null Terminator
	int minsize = MIN(strlen(cIntBin), strlen(bi.cIntBin));

	char* res = new char[size];

	for (int i=0; i < minsize; ++i)
		res[i] = cIntBin[i] & bi.cIntBin[i];

	res[minsize] = '\0';

	delete [] cIntBin;
	cIntBin = new char[size];
	strcpy(cIntBin, res);
	delete [] res;

	return *this;
}

LordJaxom

In diesem Abschnitt sehe ich keinen Fehler, der muss woanders verursacht werden.

Wieso duplizierst Du zum Schluß eigentlich res nochmal und löscht dann das Original statt res einfach cIntBin zuzuweisen?

Warum nutzt Du nicht std::string oder std::vector?

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

Du vergleichst auf Gleichheit der Werte, nicht auf Gleichheit der Objekte.
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:)

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.