2010-02-25 4 views
3

J'ai fait ma propre classe Socket, pour pouvoir envoyer et recevoir des requêtes HTTP. Mais j'ai encore quelques problèmes. Le code suivant (ma fonction de réception) est toujours buggé, et s'écrase parfois. J'ai essayé de le déboguer, mais cela doit être quelque part dans la gestion de l'arithmétique du pointeur/de la mémoire.C C++ - Classe TCP Socket: Problème de réception

int Socket::Recv(char *&vpszRecvd) 
{ 
//vpszRecvd = NULL; 
int recvsize = 0; 
char TempBuf[1024]; 
int Result = 0; 
char* temp; 


do 
{ 
    memset(TempBuf, 0, sizeof(TempBuf)); 

    Result = recv(this->sSocket, TempBuf, sizeof(TempBuf) -1, 0); 
    if (recvsize == 0) 
    recvsize = Result; 

    if (Result > 0) 
    { 
    if (vpszRecvd != NULL) 
    { 
    if (temp == NULL) 
    { 
    temp = (char*)calloc(recvsize + 1, sizeof(char)); 
    } 
    else 
    { 
    realloc(temp, recvsize + 1); 
    } 
    if (temp == NULL) 
    return 0; 

    memcpy(temp, vpszRecvd, recvsize); 
    realloc(vpszRecvd, recvsize + Result); 

    if (vpszRecvd == NULL) 
    return 0; 

    memset(vpszRecvd, 0, recvsize + Result); 
    memcpy(vpszRecvd, TempBuf, Result); 
    memcpy(vpszRecvd + recvsize, TempBuf, Result); 
    recvsize += Result; 
    } 
    else 
    { 
    realloc(vpszRecvd, Result); 

    if (vpszRecvd == NULL) 
    return 0; 

    memset(vpszRecvd, 0, Result); 
    memcpy(vpszRecvd, TempBuf, Result); 
    recvsize += Result; 
    } 
    } 
    else if ( Result == 0) 
    { 
    return recvsize; 

    } 
    else //if ( Result == SOCKET_ERROR) 
    { 
    closesocket(this->sSocket); 
    this->sSocket = INVALID_SOCKET; 
    return SOCKET_ERROR; 
    } 
} 
while(Result > 0); 

return recvsize; 
} 

Quelqu'un voit tout ce qui pourrait causer l'accident, ou que quelqu'un a un exemple mieux/plus rapide/plus petit et stable comment recevoir un paquet complet via recv()?

Je ne peux pas utiliser de chaînes, mais cela doit être fait avec des caractères.

Merci pour votre aide.

+0

memcpy (vpszRecvd, TempBuf, Résultat); Je vais le modifier maintenant. – maxedmelon

+3

Ceci a une balise 'C++'. Alors débarrassez-vous de tous les tracas de la mémoire en utilisant 'std :: vector ' pour les tampons. Utilisez sa fonction membre 'resize()' pour définir sa taille et utilisez '& v [0]' (ou '& v.begin()') si vous devez passer un 'char *' aux fonctions C: 'recv (this- Socket, & v [0], v.size(), 0) ' – sbi

+0

comme je l'ai dit, je ne peux pas le faire avec des trucs de vecteur. Cela doit être fait avec des caractères. Vecteurs/cordes serait bien ici :-( – maxedmelon

Répondre

7

Vous n'initialisez temp et, en plus de cela, votre appel à realloc est faux. Il devrait être:

temp = realloc (temp, recvsize+1); 

Lorsque vous appelez realloc que vous avez, vous jetez pas la nouvelle adresse et il y a une bonne chance que l'ancienne adresse a été libéré. Tous les paris sont désactivés lorsque vous essayez de le déréférencer. La raison realloc renvoie une nouvelle adresse parce que l'expansion du tampon peut nécessiter son déplacement si ce bloc courant est entouré dans l'arène de la mémoire (en d'autres termes, il ne peut pas simplement se développer en un bloc libre qui le suit) . Dans ce cas, un nouveau bloc sera créé dans l'arène, le contenu transféré de l'ancien bloc et l'ancien bloc libéré. Vous devez obtenir la valeur de retour de realloc dans le cas où cela se produit.

Gardez à l'esprit que realloc ne pas ont de retourner un nouveau pointeur, il peut vous donner le même pointeur si, par exemple, il y avait assez d'espace libre après le bloc pour satisfaire la nouvelle taille ou si vous re réduire la taille.

Il peut également renvoyer NULL si elle ne peut pas étendre le bloc, vous devriez faire attention pour cela aussi, surtout depuis:

temp = realloc (temp, newsize); 

se traduira par une fuite de mémoire quand il retourne NULL (il doesn pas libérer le vieux bloc).

Quelques autres:

  • vous devez rarement utiliser calloc, en particulier dans ce cas, puisque vous copiez sur la mémoire de toute façon.
  • de même, vous n'avez pas besoin de memset un bloc de mémoire à 0 si vous allez immédiatement à memcpy dessus.
  • à condition d'initialiser temp à NULL, vous pouvez simplement utiliser realloc sans le tester. C'est parce que realloc(NULL,7) est identique à malloc(7) - realloc est parfaitement capable de commencer avec un pointeur nul.
  • puisque vous n'avez pas besoin calloc, c'est pour l'éducation seulement - sizeof(char) est toujours 1 par définition.
  • Vous semblez faire énormément de copies inutiles de données.

Pourquoi ne pas commencer par quelque chose d'un peu plus simple? Maintenant, c'est totalement de ma tête donc il y a peut-être quelques bugs mais c'est au moins coupé du béhémoth qui bouge la mémoire dans la question :-) donc ça devrait être plus facile à déboguer.

Il est essentiellement décomposé en:

  • vide un message initialiser.
  • entrer une boucle infinie.
    • obtenir un segment.
    • si une erreur est survenue, libérer tout et retourner l'erreur.
    • s'il n'y a plus de segments, retournez le message en cours.
    • créer un espace pour le nouveau segment à la fin du message.
    • Si aucun espace n'a pu être créé, libérez tout et renvoyez le message vide.
    • Ajoute un segment au message et ajuste la taille du message.

et le code ressemble à ceci:

int Socket::Recv(char *&vpszRecvd) { 
    int recvsize = 0; 
    char TempBuf[1024]; 
    int Result = 0; 
    char *oldPtr; 

    // Optional free current and initialise to empty. 

    //if (vpszRecvd != NULL) free (vpszRecvd); 
    vpszRecvd = NULL; 

    // Loop forever (return inside loop on end or error). 

    do { 
     Result = recv(this->sSocket, TempBuf, sizeof(TempBuf) -1, 0); 

     // Free memory, close socket on error. 

     if (Result < 0) { 
      free (vpszRecvd); 
      closesocket(this->sSocket); 
      this->sSocket = INVALID_SOCKET; 
      return SOCKET_ERROR; 
     } 

     // Just return data and length on end. 

     if (Result == 0) { 
      return recvsize; 
     } 

     // Have new data, use realloc to expand, even for initial malloc. 

     oldPtr = vpszRecvd; 
     vpszRecvd = realloc (vpszRecvd, recvsize + Result); 

     // Check for out-of-memory, free memory and return 0 bytes. 

     if (vpszRecvd == NULL) { 
      free (oldPtr); 
      return 0; 
     } 

     // Append it now that it's big enough and adjust the size. 

     memcpy (&(vpszRecvd[recvsize], TempBuf, Result); 
     recvsize += Result; 
    } while (1); 
} 
+0

a changé maintenant. il dit 2663 octets recv'd (devrait être comme 218 ou 400 au max) et une chaîne emtpy. ne semble pas bon pour une requête HTTP normale. – maxedmelon

+0

@maxedmelon, avez-vous changé _all_ les reallocs? – paxdiablo

+0

yepp ... tous les ont changé – maxedmelon

0

J'ai eu ce problème exact très récemment.

Realloc est lent. Recv est rapide. Plusieurs centaines de reallocs par seconde vont tomber en panne.

calloc() pas seulement recvsize + 1 mais un buffer de quelques kilo-octets en plus. realloc() seulement quand le buffer serait rempli/overflow, et lui donnerait quelques kilo-octets supplémentaires sur chaque realloc(). Ce qui suit est un morceau de code que j'utilise pour ajouter des données à mon flux de sortie, mais l'entrée doit être très similaire. (Comme une note, buf_out_size est la taille du tampon alloué, buf_out_len est la quantité de données dans le tampon.)

void Netconsole::ParseOutput(int sock, std::string raw) 
    { 


     //if too small, realloc with raw.size() + BUFFSIZE. 
     if (s[sock]->buf_out_len + raw.size() > s[sock]->buf_out_size) 
     { 
      s[sock]->buf_out_size += raw.size() + BUFFSIZE; 
      s[sock]->buf_out = (char*) realloc(s[sock]->buf_out, s[sock]->buf_out_size); 
     } 

     // append new data to the end of the buffer. 
     if(s[sock]->buf_out != NULL) 
     { 
      memcpy(s[sock]->buf_out + s[sock]->buf_out_len, raw.c_str(), raw.size()); 
      s[sock]->buf_out_len += raw.size(); 

     } 
     else 
     { 
      s[sock]->ending = true; 
    #if DEBUG_PRINT_TCP 
      printf("%s TCP[%d] dies from out of memory, realloc error\r\n",Debug::MTimestamp(),sock); 
    #endif 
     } 
    } 
+0

Sonne bien.Un exemple de code? Je suppose que mon code est donc buggé, il y a plusieurs choses qui tombent en panne Je n'ai pas trouvé un seul code de réception, basé sur des caractères, qui reçoit un paquet entier – maxedmelon

+0

Pourquoi plusieurs centaines de reallocs devraient-ils avoir un deuxième crash? – sth

+0

Je suis avec sth sur celui-ci ... –