Char array übergabe

[sluz]

Halllo Leute,

ich habe diese zwei Methoden und die zicken etwas rum.

Wenn recvSocket verlassen wird, verliert die Variable data ihren Inhalt.
In der Methode selber steht in data was drin, aber hinterher nicht mehr.

Und ich weiß, es ist ein ganz schöner Misch-Masch aus C und C++, aber lassen wir es einfach so mal stehen.

void sslSocketS::recvSocket ( char *data ) {

	if ( !validSocket() ) {
		info ( "Socket ist broken!" );
		delete this;
	};

	if ( data != NULL) delete[] data;
	data = new char[ MAXRECV + 1 ];
	memset ( data, 0, MAXRECV + 1 );
	ssize_t size = ssl_read ( m_ssl, (unsigned char*) data, MAXRECV + 1 );

	if ( size == POLARSSL_ERR_SSL_PEER_CLOSE_NOTIFY ) {
		delete[] data;
		data == NULL;
		info ( "Client disconnected" );
		delete this;
	}
	else if ( size == POLARSSL_ERR_NET_CONN_RESET ) {
		delete[] data;
		data = NULL;
		warning ( "Reset by Peer!" );
		delete this;
	}
}

void ClientS::checkSocket ( ev::io &watcher, int revents ) {

	if (EV_ERROR & revents) {
        	error( "got invalid event" );
        	return;
        }

	char *buf = NULL;

        if ( revents & EV_READ ){
            	recvSocket ( buf );
	}
       	else  {
        	m_io.set ( ev::READ | ev::WRITE );
		return;
	}

        /// hier ist der buf wieder NULL?!

	readInput ( buf );

}

[Sone]

sluz schrieb:

Und ich weiß, es ist ein ganz schöner Misch-Masch aus C und C++, aber lassen wir es einfach so mal stehen.

Nie im Leben. Ersetze einfach alle char-Zeiger durch std::string, dann duerfte es sehr schnell besser gehen mit deinen Programmen

Und sowieso: EKLIG. Alles refactoren, hopp hopp!

[sluz]

Hab den Fehler nun doch endlich gefunden ... habe den Pointer im scope von recvSocket ja neu gesetzt

habe einfach die data als Rückgabewert genommen, macht weniger Probleme und ist einfach mal schöneres C.

Und ja zum Misch Masch muss ich noch dazu sagen, dass es einfach mal doof ist mit in C geschrieben Libs zu arbeiten.

[SeppJ]

sluz schrieb:

Und ich weiß, es ist ein ganz schöner Misch-Masch aus C und C++, aber lassen wir es einfach so mal stehen.

Das geht aber nicht so einfach, da sind viel zu viele Fehler drin.

Ein paar Merksätze:
- Kommt in deinem Code ein delete[] drin vor, machst du mit sehr hoher Wahrscheinlichkeit etwas falsch, da du nicht vector genommen hast.
- Kommt jemals delete this; vor, dann machst du fast sicher etwas falsch, weil du dir dadurch selber das Bein abschießt. Es gibt auf der Welt vielleicht eine handvoll Codefetzen, bei denen ein delete this; sinnvoll eingesetzt wird, die meisten davon Beispiele dafür, unter welchen verrückten Ausnahmesituationen man das doch machen kann.
- Wenn du aus einer Funktion etwas heraus gibst, was dann außerhalb mit delete gelöscht werden muss, machst du etwas falsch. Punkt.
- Ein reinterpret_cast (oder ein Cast im C-Stil, da dieser auch ein reinterpret_cast sein kann - das Problem ist, dass man das nicht so genau sehen kann) ist höchstwahrscheinlich auch ein Fehler, da deine Datentypen nicht stimmen.

Das waren deine Sünden, nun zu deinem Problem:
void sslSocketS::recvSocket ( char *data ) : Falls du hier wirklich den Wert der beim Aufruf übergebenen Variablen ändern willst, musst du die Übergabe per Referenz durchführen. Da dies eigentlich ziemlich weit vorne in einem Lehrbuch drankommen sollte, nehme ich mal an, dass du noch nicht so weit gelesen hast. Dann muss ich aber sagen, dass Socketprogrammierung viel zu schwierig für deinen Kenntnisstand ist

[SeppJ]

sluz schrieb:

Und ja zum Misch Masch muss ich noch dazu sagen, dass es einfach mal doof ist mit in C geschrieben Libs zu arbeiten.

Nur wenn du anfängst, drumherum auch C zu machen. Die Container der Standardbibliothek sind zu C-Interfaces kompatibel, um C-Pseudoobjekten kann man wunderbar "echte" Klassen schreiben, dank den C-Sprachmitteln wie Templates und Überladungen kann die Interfaces offen halten und muss sich nicht auf C-Basisdatentypen beschränken.

[sluz]

Kommt jemals delete this; vor, dann machst du fast sicher etwas falsch, weil du dir dadurch selber das Bein abschießt

Wieso sollte man damit etwas falsch machen? Wenn in dem Fall ein Socket kaputt ist, wieso auch immer, dann zerstöre ich das Object. Für jede Verbindung wird bei mir ein Object erstellt. Und damit die sich nicht ansammeln müssen ja die defekten mal raus.

Kommt in deinem Code ein delete[] drin vor, machst du mit sehr hoher Wahrscheinlichkeit etwas falsch

Ok, delete[] auf Zeile 8 ist quark, sollte man anders lösen, aber genrel muss ich doch meine erstellten array wieder löschen können. Weiß nicht wieso das falsch sein sollte.

..nehme ich mal an, dass du noch nicht so weit gelesen hast ..

Ist eher ein Problem der Überarbeitung und der Mündlichkeit

[Sone]

sluz schrieb:

Kommt jemals delete this; vor, dann machst du fast sicher etwas falsch, weil du dir dadurch selber das Bein abschießt

Wieso sollte man damit etwas falsch machen? Wenn in dem Fall ein Socket kaputt ist, wieso auch immer, dann zerstöre ich das Object. Für jede Verbindung wird bei mir ein Object erstellt. Und damit die sich nicht ansammeln müssen ja die defekten mal raus.

delete this ist *sehr* unschoenes Design. Wenn du dann noch ein Stackobjekt erstellst, wirds harig - und das kannst du nicht NICHT garantieren, ausser du bastelst dir eine factory mit ueberladenem operator new usw. BUARKS

[sluz]

Schon klar was die Stacks anbelangt und so, aber in MEINEM speziellen Fall ist es einfacher die Objekte sich selber killen zu lassen und über den Destructor sich aus dem global erreichbaren Stack zu löschen.

[Sone]

sluz schrieb:

global erreichbaren Stack

Das ist es gerade. Du darfst nur Speicher freigeben, der im Heap allokiert wurde. Das der von deiner Instanz ( *this ) aber auch mit bspw. new allokiert wurde, garantiert keiner.

[sluz]

Ach ****, habe an das richtige gedacht und das falsche geschrieben - ist natürlich alles im Heap - und darauf wurde explizit beim coden geachtet das halt kein Problem damit ensteht.

Weiß jetzt aber auch was ihr meint und worauf ihr hinaus wolltet, danke euch

[SeppJ]

sluz schrieb:

Weiß jetzt aber auch was ihr meint und worauf ihr hinaus wolltet, danke euch

Nein, du weißt, worauf Sone hinaus will, aber nicht worauf ein vernünftiger Mensch hinaus möchte : Was passiert, wenn du später auf Member zugreifst? Bämm! Außerdem: Wie erfährt der Rest des Programms, dass das Objekt sich soeben selber zerstört hat? Der Aufrufer der Funktion hat immer noch seinen Pointer auf das Objekt und wird ihn wieder benutzen. Bämm!

Von dem was du anscheinend erreichen möchtest, suchst du eine Exception, keinen Selbstmord.

[sluz]

MAN! SeppJ!

Du weißt doch nicht mal wie mein Globaler Speicher aussieht, in dem meine Objekte liegen und schon urteilst du darüber, dass er hier und da Fehler verursacht. Aber hier einmal zur Info : auf die Objekte wir über eine statische Klasse zugegriffen in der überprüft wird ob das Objekt valide ist oder nicht.

Wenn du dennoch der Meinung bist, du könntest mich eines besseren belehren, dann lass uns wo anders unterhalten und hier nicht spammen

[Sone]

SeppJ schrieb:

sluz schrieb:

Weiß jetzt aber auch was ihr meint und worauf ihr hinaus wolltet, danke euch

Nein, du weißt, worauf Sone hinaus will, aber nicht worauf ein vernünftiger Mensch hinaus möchte

Das war einer der Gruende, wieso delete this beschissen ist.

DAS ist auch einer der Gruende wieso ich mich von der Irrlicht Engine distanziert habe.

http://irrlicht.sourceforge.net/docu/_i_reference_counted_8h_source.html#l00116

[SeppJ]

sluz schrieb:

MAN! SeppJ!

[...]

Wenn du dennoch der Meinung bist, du könntest mich eines besseren belehren, dann lass uns wo anders unterhalten und hier nicht spammen

Wenn du meinst, dass du mit

auf die Objekte wir über eine statische Klasse zugegriffen in der überprüft wird ob das Objekt valide ist oder nicht.

mich auch nur irgendwie überzeugen könntest, dass hier irgendetwas sinnvolles vorgeht, dann hast du gerade das Gegenteil erreicht. Hör auf mit dem C/Java-Denk (ich bin mir nicht mir sicher, welche Programmiersprache du gerade nachzumachen versuchst): C++ ist anders!

Einen brauchbarem Lösungsvorschlag erhältst du, wenn du verrätst, was du genau erreichen möchtest.

[sluz]

Eine Lösung habe ich schon lange, willst du wirklich sehen, was ich versuche aufzubauen? Aber na gut - Ich baue einen kleinen Instant-Messenger auf Basis von libev/GTK/polarssl.

Der delete this Teil ist ein Part vom libev loop indem die SSL Sockets angenommen werden und neue Objekte ( Clients ) mit allen Socket relevanten Informationen erstellt werden.

Es werden dann aber auch wirklich alle im Heap erstellt ( weswegen ich auch delete this bedenkenlos anwenden kann ), diese Pointer lagere ich in einer Static Klasse aus, damit ich vom Frontend auf die einzelnen Clients zugreifen kann.

[Sone]

sluz schrieb:

Es werden dann aber auch wirklich alle im Heap erstellt ( weswegen ich auch delete this bedenkenlos anwenden kann ),

Das ist ein sauhaessliches Design.

[sluz]

//Nachtrag
Achso und delete this wird ja nur aufgerufen wenn der Socket hinne ist - und wenn der Socket defekt ist, hat das ganze Objekt keinen Sinn mehr.

Diese Objekte arbeiten alle selbstständig in dem ev-Loop und werden sonst nicht von irgend einer anderen Klasse oder so überwacht - daher müssen die sich selber bereinigen wenn was nicht stimmt.

UND

Ich dachte, ein halbwegs geistlich labiler Programmierer wuerde ALLERHOECHSTENS im Destruktor delete this; schreiben. Alles andere ist Wahnsinn.

Welchen Sinn macht es im destructor delete this aufzurufen wenn das Objekt in dem Moment schon zerstört wird? Gerade das erscheint mir unlogisch zu sein.

[Sone]

sluz schrieb:

Gerade das erscheint mir unlogisch zu sein.

Ja, da hab ich dank einem Denkfehler Meilenweit verfehlt. Entschuldige.
Trotzdem ist es haesslich.

[sluz]

Sone,
schau dir mal bitte den Code hier an - http://www.skitoy.com/p/writing-an-echo-server-in-libev-and-c/375

Zeile 107 & 159 wären da relevant - kannst mir dann auch gerne sagen, dass das auch hässlicher Code ist, aber dann würde ich sehr gerne mal sehen wie "schöner" Code aussieht und wie man es lösen könnte.

[Sone]

sluz schrieb:

Zeile 107 & 159 wären da relevant - kannst mir dann auch gerne sagen, dass das auch hässlicher Code ist, aber dann würde ich sehr gerne mal sehen wie "schöner" Code aussieht und wie man es lösen könnte.

Ohne das jetzt so anzusehen, wuerde ich falls eine Heap-Allokation wirklich noetig ist einfach Smart-Pointer statt rohe Pointer bei der Instanziierung nehmen.

Also statt

EchoInstance* ptr = new EchoInstance(...);
//das
std::unique_ptr<EchoInstance> ptr( new EchoInstance(...) );

Nennt sich RAII