Keskustelu: Ohjelmointikysymykset: ircbot ongelma ( C++ )

Tervehdys!

Aloitin ohjelmoimaan irc bottia oppimistarkoituksissa. Osan koodista olen kopioinut erilaisista tutoriaaleista ja osan olen kehitellyt itse. Kielihän on itselleni varsin uusi.

Tähän asti on kaikki on toiminut hyvin, botti yhdistää palvelimelle, vastaa palvelimen lähettämiin PINGeihin, liittyy kanavalle, sekä tulostaa vastaanotetun datan komentolinjaan. Myös virheiden raportoimiseen on käytetty muutama koodirivi.

Ongelma piilee nähdäkseni juuri tässä puskurin tulostuksessa. En ymmärrä miksi se tulostaa saman asian moneen kertaan. Yhtäkkiä puskurin tulostaminen myös lakkaa, mutta botti jatkaa muuten normaalia toimintaansa.

Tässä siis koko koodini nähtävillä

// Simple Ircbot
// Project started 5.7.2008

#include <windows.h>
#include <winsock.h>
#include <stdio.h>
#include <iostream>

#define NETWORK_ERROR -1
#define NETWORK_OK     0

using namespace std;

void ReportError(int, const char *);
char buffer[99999];


char user[] = "USER esimerkki d d esimerkki\r\n";
char nick[] = "NICK esimerkki\r\n";
char join[] = "JOIN #example\r\n";



int WINAPI WinMain(HINSTANCE hInst, HINSTANCE hPrevInst, LPSTR lpCmd, int nShow) {
	WORD sockVersion;
	WSADATA wsaData;
	int nret;

	sockVersion = MAKEWORD(1, 1);
	WSAStartup(sockVersion, &wsaData);
	LPHOSTENT hostEntry;

	hostEntry = gethostbyname("se.quakenet.org");

	if (!hostEntry) {
		nret = WSAGetLastError();
		ReportError(nret, "gethostbyname()");

		WSACleanup();
		return NETWORK_ERROR;
	}


	// Create the socket
	SOCKET theSocket;

	theSocket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);

	if (theSocket == INVALID_SOCKET) {
                  nret = WSAGetLastError();
		          ReportError(nret, "socket()");

		WSACleanup();
		return NETWORK_ERROR;
	}


	SOCKADDR_IN serverInfo;
	serverInfo.sin_family = AF_INET;
	serverInfo.sin_addr = *((LPIN_ADDR)*hostEntry->h_addr_list);
	serverInfo.sin_port = htons(6667);


	// Connect to the server
	nret = connect(theSocket, (LPSOCKADDR)&serverInfo,sizeof(struct sockaddr));

	if (nret == SOCKET_ERROR) {
		nret = WSAGetLastError();
		ReportError(nret, "connect()");

		WSACleanup();
		return NETWORK_ERROR;
	}

	else {
         cout << "Succesfully connected" << endl;

         }

         // Send USER and NICK with their params to server
         send(theSocket, user, strlen(user), 0);
         send(theSocket, nick, strlen(nick), 0);

  int a;
  char *motd;
  char *p, tokit[30];
  char *j;
  char poulet[30];



    while(theSocket) {

                  // To print received data to console
                   nret = recv(theSocket, buffer, sizeof(buffer), 0);
                   cout << ("%s\r\n",buffer);

         // See if Message of the Day is received and join channel
         motd = strstr (buffer, "/MOTD");

         if (motd)
         {
                  send(theSocket, join, strlen(join), 0);
                  }

                   // To see if we received PING
                   p = strstr (buffer, "PING :");
                   if (p)
                   {

                         for (p += 6, a = 0; *p && *p != '\n'; p++, a++)
                             tokit[a] = *p;
                         tokit[a] = 0;

                                   // And answer to it
                                   sprintf (poulet, "PONG :%s\n", tokit);
                                   send (theSocket, poulet, strlen (poulet), 0);

                                   printf ("%s\n",poulet);

                   }

}

return 0;
}

// Messagebox for reporting errors

void ReportError(int errorCode, const char *whichFunc) {
   char errorMsg[92];

   ZeroMemory(errorMsg, 92);
   sprintf(errorMsg, "Call to %s returned error %d!", (char *)whichFunc, errorCode);
   MessageBox(NULL, errorMsg, "socketIndication", MB_OK);
}

Eli jos saisin vastauksia noihin kahteen kysymykseen :) Lisäksi mielelläni kuuntelen muita ohjeita / neuvoja koodin parantamiseksi.

Kiitos.

Pistä bufferiksi string bufferi, niin silloin mahtuu loputtomiin dataa sinne... Ja muutkin merkkijonot stringeiksi...

vehkis91 kirjoitti:

Pistä bufferiksi string bufferi, niin silloin mahtuu loputtomiin dataa sinne... Ja muutkin merkkijonot stringeiksi...

Mitenkäs tämä tapahtuisi käytännössä?

#include <string>

//vaihdat näin
char bufferi[]-> string bufferi

string user = "USER esimerkki d d esimerkki\r\n";
string nick = "NICK esimerkki\r\n";
string join = "JOIN #example\r\n";

Jos tuo bufferi ei toimi, niin pistä pelkät merkkijonot sitten stringeisksi.

Kokeilin jo tuota aikaisemmin, mutta koska antoi erroria niin päätin kysyä kuinka tuo käännös tapahtuisi :)

Tässä virheet mitä kääntäjä ilmoitti:

In function `int WinMain(HINSTANCE__*, HINSTANCE__*, CHAR*, int)':

cannot convert `std::string' to `char*' for argument `2' to `int recv(SOCKET, char*, int, int)'

no matching function for call to `strstr(std::string&, const char[6])'

pistä siihen:

recv(theSocket, buffer.c_str(), sizeof(buffer), 0);

Tuon const_castin perusteella string-luokan c_str-metodi palauttaa vakioviittauksen stringin sisältämään merkkidataan C-muodossa. Tällöin datalle on luultavasti varattu vain sen verran tilaa, minkä se todellisuudessa vie. Tähän taulukkoon ei ole tarkoitus kirjoittaa mitään, mistä kertoo mm. se, että vehkis joutuu käyttämään const_castia saadakseen virityksen kääntäjästä läpi. Vehkiksen esimerkin string on ainakin alussa tyhjä, joten recv-funktiolle annettavan merkkitaulukon koko on nolla. Sizeof(buffer) taas on string-luokan, ei siis parametrinä annettavan merkkitaulukon, koko.

Joku virhe varmaan unohtui, mutta joku muu voi täydentää.

Yhteenvetona: vehkiksen koodi EI tässä tilanteessa toimi.

EDIT 1:

Recv-funktion palauttama merkkijono ei sisällä merkkijonon loppua osoittavaa nollamerkkiä. Se on lisättävä itse.

No mites sitten laitetaan silleen että käytetään stringiä bufferina?

Edit:

recv(theSocket, const_cast<char *>(buffer.c_str()), sizeof(buffer.c_str()), 0);?

vehkis91 kirjoitti:

No mites sitten laitetaan silleen että käytetään stringiä bufferina?

Ei tule ainakaan mieleen, miten tuollainen voisi onnistua. Helpoimmalla varmaan pääsee kirjoittamalla oman recv-wrapperin, joka lukee merkkejä vaikkapa rivinvaihtoon asti ja muodostaa niistä stringin.

eli lyhyesti käytetään char-taulukko bufferina...

En näe erityistä syytä stringin käyttöön. IRC-protokolla jo itsessään rajoittaa rivin pituutta, ja puskurisi on aivan tarpeettoman iso.

Kuvittelisin, että recv ei aseta tekstin loppuun tarvittavaa nollamerkkiä. Aseta siis vastaanottamisen jälkeen se itse:

buffer[nlen] = 0;

Ehto "while (theSocket)" on huono, koska tuon muuttujan arvo ei muutu silmukassa.

Koodin parantamisesta: Asettelua ja sisennystä voisit ainakin parantaa, koodi on vaikeaselkoista, kun lohkot eivät ole yhtenäisesti sisennettyjä. Älä myöskään käytä turhan päiten Windowsin outouksia kuten WinMain-funktiota ja muuta sellaista, kun niiden käytölle ei ole erityistä perustetta, vaan tee ohjelmastasi helposti muillekin käyttöjärjestelmille käännettävä.

KeKimmo kertoikin jo olennaisen tästä string-häsellyksestä. Vehkis voisi ehkä hieman harkita ennen vastaamista, ei millään pahalla. Näitä viallisia purkkavinkkejä tahtoo sadella aika tiheään.

Mietin kyllä, ja minulle on aina sanottu, että pitää käyttää mahdollisimman paljon stringejäja mahdollisimman vähän char-taulukoita. Joten sori vaan, otan opikseni.

Juu, tosiaan vehkiksen kanssa tätä eilen pohdiskeltiin ja päädyttiin siihen tulokseen, että aloitan saman projektin käyttäen SDL_net:iä.

lainaus:

pitää käyttää mahdollisimman paljon stringejäja mahdollisimman vähän char-taulukoita

Täällä ( http://www.nic.funet.fi/c opas/stdlib.html#string ) taasen neuvotaan käyttämään mahdollisimman vähän stringejä ja suosia enemmänki char-taulukoita :)

Noh, en tiedä tästä, jospa joku joka tietää voisi vähän valaista?

Mutta kiitokset jo tässä vaiheessa vastanneille.

Tuolla kyllä sanotaan, että ei kannata käyttää char-taulukoita... :P

"C:ssä merkkijonot ovat char-taulukoita. Ja niinhän ne pohjimmiltaan ovat C++:ssakin. Merkkijonoja käytettäessä on hyvä muistaa, että niitä ei pidä mennä käyttämään.Vältä niitä kuin ruttoa."

Se poikkeus tuohon sääntöön on juuri tämä ulkoisten rajapintojen känssä säheltäminen. Niiden kanssa voi vaan olla pakko.

Okei, pidän mielessä.

Ja tämä poikkeus johtuu tarkemmin siitä, ettei Windowsille (kuten todella monelle muullekaan järjestelmälle/kirjastolle) ole erillistä C++-ohjelmointirajapintaa, vaan joudutaan käyttämään C-kielelle tarkoitettua APIa. C-kielisiä rajapintoja joutuu käyttämään C++-ohjelmoinnissa yleensä paljon, mikä on hyvä syy osata C++:n lisäksi myös sujuvasti C:tä.

Socketeille toki löytyy myös "epävirallisia" C++-wrappereita
http://www.google.com/search?q=c++ socket

jotka kannattaa ottaa käyttöön tai kirjoittaa itse vastaava, ennen kun pää hajoaa kivikautisten socket-APIen kanssa. Tai ihan hyviähän ne on, jos korkeampi abstraktiotaso ei vaan kelpaa...

Tein oman botin käyttämällä SDL_nettiä ja siinä ei ole mitään tuon kaltaisia ongelmia... Koodi on melkeen suoraan käännetty tuosta koodista winsock -> SDL_net.

Stringit ovat C++ koodarille ylivertainen char taulukoihin verrattuna, turvallisuutensa ja käyttömukavuutensa ansiosta. oikeastaan ainoa syy käyttää chareja, on juuri tämä vaadittu C-yhteensopivuus, sekä se, että joissain systeemeissä ei muisti riitä stringien (eikä varsinkaan STL containereiden) käyttöön. Ja yksi mahdollinen syyhän on tietysti, se, että osa meistä (kuten minä) ovat vanhoja jääriä, joilla on luontainen inho kaikkea "uutta" kohtaan...

Mutta chareja voi kyllä käyttää kun on tarkkana, että.
1. Et ikinä ylikirjoita varaamaasi tilaa!. Huom, char arrayn päättävä NULL merkki vie yhden tavun ylimääräistä tilaa!
2. Et ikinä unohda NULL merkkiä char taulusi lopusta. Suurín osa C:n str* funktioista ottaa argumenttinaan char arrayn alkuosoitteen, ja lukee muistia kunnes ensimmäinen NULL merkki löytyy, tulkiten kaiken tällä välillä tarkoitetuksi char taulukoksi. Jos NULL merkki unohtuu, lukee (tai kirjoittaa!!) str* funktiot satunnaisen pätkän muistia - kunnes kohtaavat ensimmäisen NULL merkin, tai kunnes käyttöjärjestelmä huomaa prosessin kirjoittelevan/lukevan mitä sattuu muistialuetta, ja tappaa sen antaen Ah niin Lohduttavan ilmoituksen:
Segmentation Fault.

C++ stringien kanssa näistä huolenaiheista pääsee osaksi eroon...

Lisäksi ottaen huomioon, että kaikissa järjestelmissä muistin määrä on rajallinen, tekee seuraava pointti Stringeistä aivan ylivertaisen...

vehkis91 kirjoitti:

Pistä bufferiksi string bufferi, niin silloin mahtuu loputtomiin dataa sinne..

:D