[C++]IrcClient, server geeft geen ping

[Verwijderd]

Ik ben begonnen met een IRC Client te maken, leek me wel es leuk enzow
Source: Hiero

De thread die je ziet leest dus de hele tijd en checkt of de bestaande buffer ongelijk is aan de nieuwe inhoud, dus of er iets binnengekomen is.

Ik stuur netjes mijn NICK en USER zooi zoals dat hoort volgen de RFC, maar krijg nix terug, ik zou toch eigenlijk een PING moeten krijgen of niet ?

Verder zul je bij het runnen zien dat er streepjes gezet worden (door de thread)
die streepjes houden opeens op als er geschreven wordt naar de socket, iets dat ook niet helemaal klopt dus. Maar als er vervolgens een ERROR binnenkomt (ping timeout na 30 secs ofzow) gaat ie wel verder met de thread.

voag???, ja sehr voag.

Mijn vraag is zouden jullie es een al dan niet deskundige blik er op kunnen werpen en me zeggen wat er aan scheelt??

[.oisyn]

De thread die je ziet leest dus de hele tijd en checkt of de bestaande buffer ongelijk is aan de nieuwe inhoud, dus of er iets binnengekomen is.

beetje kromme manier, denk je ook niet? Je kunt beter gewoon het resultaat van de recv functie gebruiken om te zien of er wat ontvangen is. Dan weet je bovendien ook gelijk hoeveel er ontvangen is


en dit gaat natuurlijk ook niet goed:

C++:

void IrcClient::SendStr(char* sendstr)
{
    cout<<"--Sending--"<<endl<<sendstr;
    send(sock,sendstr,sizeof(sendstr),0);
}

hier verstuur je altijd 4 bytes. Mag jij uitzoeken waarom


Ho nog iets, in de functie ReadThread

C++:

memset(&(theClient->buffer),'\0',1000);

theClient->buffer is een pointer. Wat jij nu doet is het adres van de pointer overschrijven met 1000 keer een 0 character. Aangezien een pointer maar 4 bytes groot is overschrijf je dus potentieel iets wat helemaal niet de bedoeling is. Je moet gewoon schrijven naar waar de pointer naar wijst, dus zo:

C++:

memset(theClient->buffer,'\0',1000);

Ik zie dat je meer van die address-of operators (de & dus) gebruikt bij arrays. Dat gaat bij arrays met een statische grootte toevallig goed, maar beter kun je ze gewoon helemaal niet gebruiken

[ Voor 73% gewijzigd door .oisyn op 26-06-2003 12:05 ]

[curry684]

IRC protocol als gespecificeerd in deze RFC schrijft voor dat er helemaal geen pings volgen maar alleen numerieke antwoorden. Dump gewoon alle data die je terugkrijgt (mijn eigen IRC-botje werkt nl. op minder code dan jij hebt perfect, maar die code heb ik thuis liggen).

[curry684]

hier verstuur je altijd 4 bytes. Mag jij uitzoeken waarom

Idd, en daardoor komt de CrLf pair nooit aan, waardoor de server nooit een commando ziet, waardoor hij na 30 seconden netjes zal timeouten

[.oisyn]

C++:

IrcClient::~IrcClient()
{
  GetExitCodeThread(hThread,&threadXitId);
  if(TerminateThread(hThread,threadXitId))cout<<"Thread Destruct";
  delete[] buffer;
}

da's natuurlijk ook niet zo netjes. Beter is om een waarde te zetten waardoor de thread kan zien dat ie moet stoppen, zodat ie zichzelf netjes op kan ruimen. TerminateThread () mag eigenlijk alleen in het ergste geval gebruikt worden, dus als een thread gewoonweg niet meer reageert

[curry684]

.oisyn bedoelt dus dat je CreateEvent, SetEvent, WaitForSingleObject en WaitForMultipleObjects moet gebruiken om de thread te signallen dat ie moet stoppen, waarna de thread automatisch zichzelf afsluit, en je met WaitForSingleObject in die destructor kunt wachten tot de thread clean afgesloten is

[Verwijderd]

.oisyn schreef op 26 June 2003 @ 12:01:
[...]


beetje kromme manier, denk je ook niet? Je kunt beter gewoon het resultaat van de recv functie gebruiken om te zien of er wat ontvangen is. Dan weet je bovendien ook gelijk hoeveel er ontvangen is


en dit gaat natuurlijk ook niet goed:

C++:

void IrcClient::SendStr(char* sendstr)
{
    cout<<"--Sending--"<<endl<<sendstr;
    send(sock,sendstr,sizeof(sendstr),0);
}

hier verstuur je altijd 4 bytes. Mag jij uitzoeken waarom


Ho nog iets, in de functie ReadThread

C++:

memset(&(theClient->buffer),'\0',1000);

theClient->buffer is een pointer. Wat jij nu doet is het adres van de pointer overschrijven met 1000 keer een 0 character. Aangezien een pointer maar 4 bytes groot is overschrijf je dus potentieel iets wat helemaal niet de bedoeling is. Je moet gewoon schrijven naar waar de pointer naar wijst, dus zo:

C++:

memset(theClient->buffer,'\0',1000);

Ik zie dat je meer van die address-of operators (de & dus) gebruikt bij arrays. Dat gaat bij arrays met een statische grootte toevallig goed, maar beter kun je ze gewoon helemaal niet gebruiken

woeiiii zow heej dat is effe je code laten verbeteren.
die sizeof heb ik gecopypaste, niet aan gedacht om m verder om te pleuren. Die memset(&(theClien die was ik al aant verbeteren.

En wat betreft de & operators heb je gelijk.

De thread however ben ik het niet zo 100% mee eens, die zorgt er namelijk voor dat alle zooi die binnenkomt ook daadwerkelijk in de buffer gepleurd wordt. Zodat ik er niet expliciet om hoef te vragen

maarja
ik heb m'n ping wij danken U

[.oisyn]

Ik heb me trouwens vergist, jouw buffer was niet dynamisch

C++:

class IrcClient
{
  private:
    SOCKET sock;
    sockaddr_in addr;
    IrcClient();
    unsigned long threadXitId;
    HANDLE hThread; 
  public:
    char buffer[1000];
    IrcClient(char* Address,unsigned short Port);
    IrcClient(const IrcClient&);
    ~IrcClient();
    void SendStr(char* sendstr);
    SOCKET getSock(){return sock;};
    void Auth(char* Nick,char* User, char* Host,char* Server,char* RealName);
} ;

De verwarring was door de delete[] buffer die je in je destructor hebt staan. Je mag natuurlijk niet iets deleten dat je ook niet met new hebt gealloceerd

Sowieso is het gebruik van buffers met een vaste grootte vragen om problemen

[curry684]

Dat de destructor van die class de thread niet netjes stopt maakt trouwens weinig uit gezien je in je main functie vergeet de class te deleten waardoor die destructor nooit aangeroepen wordt.

[.oisyn]

curry684 schreef op 26 June 2003 @ 12:12:
.oisyn bedoelt dus dat je CreateEvent, SetEvent, WaitForSingleObject en WaitForMultipleObjects moet gebruiken om de thread te signallen dat ie moet stoppen, waarna de thread automatisch zichzelf afsluit, en je met WaitForSingleObject in die destructor kunt wachten tot de thread clean afgesloten is

een simpele volatible bool variabele werkt ook wel. Ipv de while (true) doe je gewoon iets als while (running), en als je wilt stoppen zet je running gewoon op false.

Het netste is om in de destructor dan running op false te zetten, dan kijken of de thread binnen een paar seconde stopt, en zo niet alsnog een TerminateThread te doen

Overigens is het gebruik van CreateThread () en ExitThread () sterk af te raden als je gebruik maakt van de C/C++ runtime. Gebruik hiervoor de functies _beginthread () en _endthread ()

[curry684]

Het netste is om in de destructor dan running op false te zetten, dan kijken of de thread binnen een paar seconde stopt, en zo niet alsnog een TerminateThread te doen

Wat je dus zo doet:

C++:

// Wait 10 seconds for completion
if(WaitForSingleObject(ThreadHandle, 10000) != WAIT_OBJECT_0)
  TerminateThread(ThreadHandle);

[curry684]

Zoepnek in z'n openingspost:

Mijn vraag is zouden jullie es een al dan niet deskundige blik er op kunnen werpen en me zeggen wat er aan scheelt??

Ach de C++ kant van de Devschuur crew komt ook weer eens aan z'n trekken