Socket-Verhalten

zuckerlie schrieb:

Hallo Leute,

int Socket::RecvHeader(char * headerbuf){

this->SockReturn = 0;

while((this->SockReturn = ::recv(this->sock,headerbuf,42,0)) < 42)
{
::Sleep(1);
};

return this->SockReturn;
};

Genau da ist der Fehler. Wenn Du weniger als 42 Bytes empfängs, liest Du noch mal 42 Bytes. Und das noch dazu an den Anfang deines Puffers. Bekommst Du beispielsweise 2 Bytes beim ersten Aufruf und 42 beim nächsten, hast Du die Bytes 2-43 (also von 0 an gezählt) in Deinem Puffer, statt 0-41, wie Du es erwartest. Bei den Nutzdaten machst Du das dann korrekt.

Und so ganz nebenbei: ein Sleep ist immer die falsche Lösung. Nimm select oder poll.

Belli

zuckerlie schrieb:

while((this->iRecv = ::recv(this->sock,recvbuf,NUTZDATENLAENGE,0)) < NUTZDATENLAENGE)
{
::Sleep(1);
};

Angenommen, Du erwartest die von mir weiter oben erwähnten 10000 Bytes, die aber nur portionsweise ankommen. Dann bekommt this->iRecv nacheinander die Werte 2000, 3000 und 5000 - bleibt aber immer kleiner als 10000 und deshalb kommt das Programm nicht aus dieser Schleife heraus.

Und was soll überhaupt das Sleep(1) hier?

zuckerlie

So, ich melde mich zurück.

Erstmal vielen Dank.

Ich habe schon oft von diesem Problem gelesen, aber nicht gedacht, dass es in diesem Fall zutrifft.

Wie ich bereits sagte, habe ich bei Codeproject gelesen, dass das recv in die Bedingung der While-Schleife gesetzt wurde und somit keine mehrfachen Aufrufe innerhalb der Schleife nötig sind. Das Sleep(1) innerhalb dieser Schleife war um ggf. sonst eine Millisekunde zu warten und dann nochmal zur Bedingung zu gehen.

Ich habe es nun so umgesetzt:

int connectsocket::RecvPayload(char * recvbuf, int iPayload)
{
    this->iReturnPayload = 0;
    this->iReturnPayload = ::recv(this->sock, recvbuf,iPayload, 0);
    //recvbuf innerhalb der main:
    // char * recvbuf = new recvbuf[Payload]
    while(this->iReturnPayload != iPayload)
    {

this->iReturnPayload += ::recv(this->sock, recvbuf+this->iReturnPayload ,iPayload, 0);

    };

    return this->iReturnPayload;
}

Belli

Falls die Gegenstelle weitersendet, willst Du hier aber nur die für diese Übertragung gewünschte Datenmenge lesen, deshalb:
this->iReturnPayload += ::recv(this->sock, recvbuf+this->iReturnPayload ,iPayload - iReturnPayload, 0)

314159265358979

Bitte keine ungarische Notation und mach die ganzen this-> weg.

_--

sockets sind schwer... zumindest für mich! und wenn ich mich jetzt nicht täusch, sind in deinem so viele fehler (was passiert bei ::recv rückgaben von 0 bzw -1 was ja kleiner als dein NUTZDATENLAENGE ist?), dass ich dir empfehle eine fertige library zu nehmen.

okay, evtl. wird da auch dann ganz was anderes gemacht, kenn ja ::recv nicht und will dir jetzt nichts unterstellen

@edit warte, das ist windows... da ists statt -1 SOCKET_ERROR, oder

cooky451

Nö, sockets sind eigentlich ziemlich simpel.
@zuckerlie
Für dich gibt es zwei Lösungen: select() oder Threads. (Oder poll(), aber das dürfte Windows nicht haben.) (Du kannst den Socket auch select()en nachdem er verbunden wurde, um zu testen ob du etwas Lesen kannst.)

Und ja, du musst natürlich den Rückgabewert von recv() überprüfen bevor du das irgendwo drauf addierst, ist doch klar. Und hör auf Pi.

zuckerlie

Oh man oh man, vielen Dank Leute.

Ihr habt mir sehr geholfen.

@Belli, da hast du recht. Das habe ich natürlich auch nicht bedacht.
//edit, aber das ändert doch nichts so wirklich, da ich ja sowieso nach einer gewissen menge aufhöre. sollte doch selbst so ohne probleme gehen?!

@Cooky, ja der Rückgabewert wird geprüft. Habe ich schon programmiert. Ja, ich nutze schon select() um zu schauben, ob ich lesen kann.

@Pi, spricht was dagegen?

Vielen Dank Euch

314159265358979

zuckerlie schrieb:

@Pi, spricht was dagegen?

Spricht was dafür?

zuckerlie

314159265358979 schrieb:

zuckerlie schrieb:

@Pi, spricht was dagegen?

Spricht was dafür?

Übersicht?!

314159265358979

xDDDDDD
Spinner.

Belli

zuckerlie schrieb:

@Belli, da hast du recht. Das habe ich natürlich auch nicht bedacht.
//edit, aber das ändert doch nichts so wirklich, da ich ja sowieso nach einer gewissen menge aufhöre. sollte doch selbst so ohne probleme gehen?!

Nein. Stell Dir vor, Du bist mit dem Lesen des Päckchens noch nicht ganz fertig. Sagen wir, Dir fehlen noch 3000 Byte. Nun sendet die Gegenstelle wieder 10000 Byte. Von Deinem Socket sind jetzt insgesamt 13000 Byte lesbar, wovon 3000 Byte noch zu dem aktuellen Päckchen gehören, und auch nicht mit den nächsten 10000 zusammengewürfelt werden dürfen.
Nun kommt Dein

this->iReturnPayload += ::recv(this->sock, recvbuf+this->iReturnPayload ,iPayload, 0);

wobei iPayload ja die ganze Zeit den Wert 10000 hat.
Jetzt liest dieser Aufruf von recv möglicherweise mehr als die noch benötigten 3000 Byte, nämlich irgendwas zwischen 1 und 10000, weil Du 10000 (iPayload) als Maximalwert angegeben hast.
Alles klar?

Zu allem Überfluss wird jetzt nicht mal mehr die Schleife verlassen, weil Du als Bedingung '!=' drin hast, iReturnPayload jetzt aber größer als iPayload geworden ist ...

zuckerlie

Oh vielen Dank. So habe ich das gar nicht gesehen.

Wieder mal was gelernt Danke

@Pi, laber nicht so nen Müll,sondern begründe doch mal deine erste Aussage..

Ethon

this-> ist in praktisch allen seinen Fällen komplett redundant. Redundanter Code ist schlecht.

314159265358979

zuckerlie schrieb:

@Pi, laber nicht so nen Müll,sondern begründe doch mal deine erste Aussage..

Das ist zu offensichtlich für eine Begründung. Recherchieren kannst du selber.

_--

cooky451 schrieb:

Nö, sockets sind eigentlich ziemlich simpel.

schön dass ich der einzige depp bin... und feststelle, wenn ich das internet nach brauchbaren tuts/code durchforste und verbuggten einfachsten code finde, dass die anderen auch nicht klüger sind :p

@edit: aber hey, wir sind ja schon bei 30 von 300 sloc, die für eine halbwegs vernünftige implementation nötig sind...
@edit2: und den einen fehler, den wir uns in 300 sloc erlauben dürfen haben wir auch schon mit eingebaut - toll

cooky451

Du findest zu jedem Thema verbugten Code. Ändert aber nichts daran, dass man einfach immer testen muss, ob die Funktion erfolgreich war. Soo unglaublich komplex finde ich das jetzt nicht.

_--

cooky451 schrieb:

Du findest zu jedem Thema verbugten Code. Ändert aber nichts daran, dass man einfach immer testen muss, ob die Funktion erfolgreich war. Soo unglaublich komplex finde ich das jetzt nicht.

socket und threads sind einfach etwas, was total beschissen zu testen ist.

zuckerlie

Ethon schrieb:

this-> ist in praktisch allen seinen Fällen komplett redundant. Redundanter Code ist schlecht.

Danke, da werde ich mal recherchieren.

cooky451 schrieb:

Du findest zu jedem Thema verbugten Code. Ändert aber nichts daran, dass man einfach immer testen muss, ob die Funktion erfolgreich war. Soo unglaublich komplex finde ich das jetzt nicht.

Es ist nicht komplex. Es ist sogar so einfach, dass das drumherum einfach noch einiges machen muss. Daher sollte man eher geeignete Libraries verwenden, um das zu machen. Wir sehen hier in diesem Thread, was alles falsch gemacht werden kann.

Es gibt auch noch das EINTR. Wenn recv über ein Signal unterbrochen wird, sollte ich es möglichst noch mal aufrufen.

Auch wäre es sinnvoll, Pufferung einzubauen. Statt 42 Bytes zu lesen, wäre es sinnvoll, beispielsweise 8k zu lesen und 42 Bytes zu verarbeiten. Wenn ich dann noch weitere Daten brauche, finde ich sie bereits im Puffer und spare einen Systemaufruf. Und das kann die Performance signifikant verbessern.

Und Dein Hinweis, dass man immer testen muss, ob die Funktion erfolgreich war, ist nur halb richtig. Ich muss natürlich auf alle Fehler reagieren. Wenn ich aber eine passende Library habe, dann kann sie im Fehlerfall eine Exception auslösen. Dann kann ich den Fehler automatisch auf einer höheren Ebene verarbeiten.

Und tut mir einen Gefallen: don't feed the trolls. Spätestens wenn einer "Spinner" sagt, sollte man ihn ignorieren. Das ist kein respektvolles Verhalten gegenüber anderen Mitgliedern hier.