Libérer la mémoire et fonction recvfrom

Libérer la mémoire et fonction recvfrom - C - Programmation

Marsh Posté le 05-01-2007 à 10:12:58    

Bonjour
 
J'ai écris un programme me permettant de faire des pings sous Linux, utilisant des RAW Socket ICMP.
A chaque ping, la fonction Ping(IP) est appellée. Le code a été repris en grande partie sur Internet, je n'ai pas une connaissance poussée du langage C, donc j'ai du mal à trouver comment libérer toute la mémoire utilisée par la fonction Ping que voici :
 

Code :
  1. int Ping(char * IP)
  2. {
  3. int failed = 0;
  4. int sock,optval;
  5. char *packet,*buffer;
  7. struct icmpheader * icmp;
  9. struct sockaddr_in peer;
  10. struct ipheader * ip;
  12. ip     = (struct ipheader *) malloc(sizeof(struct ipheader));
  13. icmp = (struct icmpheader *) malloc(sizeof(struct icmpheader));
  14. packet = (char *)     malloc(sizeof(struct iphdr) + sizeof(struct icmphdr));
  15. buffer = (char *)     malloc(sizeof(struct iphdr) + sizeof(struct icmphdr));
  17. ip   = (struct ipheader *) packet;
  18. icmp = (struct icmpheader *) (packet + sizeof(struct ipheader));
  20. // Creating IP header
  22. ip->ip_hl = 5;
  23. ip->ip_v  = 4;
  24. ip->ip_tos = 0;
  25. ip->ip_len = sizeof(struct ipheader) + sizeof(struct icmpheader);
  26. ip->ip_id = (int)rand();
  27. ip->ip_off = 0;
  28. ip->ip_ttl = 64;
  29. ip->ip_p = IPPROTO_ICMP;
  30. ip->ip_sum = 0;
  31. char * IpSrc = (char *) malloc(32*sizeof(char));
  32. strcpy(IpSrc,(char *)GetLocalIP);
  33. if(strstr(IpSrc,"0.0.0.0" ))
  34. {
  35.  // FAILED (unable to resolve local IP)
  37.  failed = 1;
  38. }
  39. else
  40. {
  41.  ip->ip_src = inet_addr(IpSrc);
  42.  ip->ip_dst = inet_addr(IP);
  44.  // Creating socket
  46.  sock = socket(AF_INET,SOCK_RAW,IPPROTO_ICMP);
  47.  if(sock == -1)
  48.  {
  49.   // FAILED (creating socket)
  51.   failed = 2;
  52.  }
  53.  else
  54.  {
  55.   // Setting socket options
  57.   int ret = setsockopt(sock,IPPROTO_IP,IP_HDRINCL,&optval,sizeof(int));
  58.   if(ret != 0)
  59.   {
  60.    // FAILED (setting socket options)
  62.    failed = 3;
  63.   }
  64.   else
  65.   {
  66.    // Creating ICMP header
  68.    icmp->icmp_type = 8;
  69.    icmp->icmp_code = 0;
  70.    icmp->icmp_cksum = 0;
  71.    icmp->icmp_cksum = in_cksum((unsigned short *)icmp,sizeof(struct icmpheader));
  72.    icmp->icmp_id = 0;
  73.    icmp->icmp_seq = 0;
  75.    peer.sin_family = AF_INET;
  76.    peer.sin_addr.s_addr = inet_addr(IP);
  78.    // Sending ICMP packet
  80.    ret = sendto(sock,packet,ip->ip_len,0,(struct sockaddr *)&peer,sizeof(struct sockaddr));
  81.    if(ret != ip->ip_len)
  82.    {
  83.     // FAILED (not all packet sent)
  85.     failed = 4;
  86.    }
  87.    else
  88.    {
  89.     // Waiting for ICMP reply
  91.     ret = recv(sock,buffer,sizeof(struct ipheader)+sizeof(struct icmpheader),0);
  92.     if(ret != (sizeof(struct ipheader)+sizeof(struct icmpheader)))
  93.     {
  94.      // FAILED (ICMP packet received but may be corrupted (didn't receive expected size))
  96.      failed = 5;
  97.     }
  98.    }
  99.   }
  100.          close(sock);
  101.  }
  102. }
  104. free(IpSrc);
  105. free(buffer);
  106. free(packet);
  108. return failed;
  109. }


Lorsque le programme tourne en boucle, la mémoire utilisée grandie petit à petit...
Merci de me dire ce que je dois ajouter à la fin de la fonction pour que toute la mémoire soit libérée :hello:


Message édité par XK le 12-01-2007 à 11:19:44
Reply

Marsh Posté le 05-01-2007 à 10:12:58   

Reply

Marsh Posté le 05-01-2007 à 10:17:53    

Tu as 5 malloc et 3 free, il en manque 2 ... ip et icmp

Reply

Marsh Posté le 05-01-2007 à 10:33:06    

J'ai essayé de les ajouter mais à l'exécution j'ai une belle erreur à chaque fois... je ne sais pas si il y a un ordre à respecter, si j'essaye dans l'ordre inverse de déclaration ça ne fonctionne pas.
D'ailleurs si je ne fais que "free(icmp);" j'ai aussi l'erreur.
L'erreur est du type " *** glibc detected *** munmap_chunk() : invalid pointer ".

Reply

Marsh Posté le 05-01-2007 à 10:37:36    

C'est normal.

 

Le malloc :

icmp = (struct icmpheader *) malloc(sizeof(struct icmpheader));

 

Puis, quelques lignes en dessous :

icmp = (struct icmpheader *) (packet + sizeof(struct ipheader));

 

Le pointeur vers la zone mémoire que tu as allouée s'est donc paumé dans la nature, et ce que tu donnes à free() ne correspond plus à ce que t'avais retourné malloc().
Tu essaies donc de libérer une zone mémoire qui n'a pas à l'être, tandis que celle que tu devais désallouer est quelque part dans ton segment.

Message cité 1 fois
Message édité par Elmoricq le 05-01-2007 à 10:37:47
Reply

Marsh Posté le 05-01-2007 à 10:44:49    

Elmoricq a écrit :

C'est normal.
 
Le malloc :

icmp = (struct icmpheader *) malloc(sizeof(struct icmpheader));


 
Puis, quelques lignes en dessous :

icmp = (struct icmpheader *) (packet + sizeof(struct ipheader));


 
Le pointeur vers la zone mémoire que tu as allouée s'est donc paumé dans la nature, et ce que tu donnes à free() ne correspond plus à ce que t'avais retourné malloc().  
Tu essaies donc de libérer une zone mémoire qui n'a pas à l'être, tandis que celle que tu devais désallouer est quelque part dans ton segment.


Un cas typique de 'fuite mémoire'...


---------------
Des infos sur la programmation et le langage C: http://www.bien-programmer.fr Pas de Wi-Fi à la maison : http://www.cpl-france.org/
Reply

Marsh Posté le 05-01-2007 à 10:59:55    

Merci pour vos réponses rapides :)
 
Je viens d'essayer en supprimant les deux lignes :

Code :
  1. ip     = (struct ipheader *) malloc(sizeof(struct ipheader));
  2. icmp = (struct icmpheader *) malloc(sizeof(struct icmpheader));


 
Ainsi il n'y a plus de fuite mémoire :jap:  
 
Mais est-ce que c'était important d'allouer la mémoire à cet endroit? :??:

Message cité 2 fois
Reply

Marsh Posté le 05-01-2007 à 11:03:15    

XK a écrit :

Mais est-ce que c'était important d'allouer la mémoire à cet endroit? :??:

 

"Allouer de la mémoire" signifie, très clairement, "dégager un espace libre pour y stocker des données".
C'est ce que fait malloc(), c'est pourquoi tu lui indiques la taille dont tu as besoin, et le pointeur retourné est l'adresse de départ de cette mémoire fraîchement allouée.

 

Ici, visiblement, de cet espace mémoire tu n'en as pas l'usage. Pourquoi en avoir alloué ?


Message édité par Elmoricq le 05-01-2007 à 11:04:07
Reply

Marsh Posté le 05-01-2007 à 11:05:16    

XK a écrit :

Je viens d'essayer en supprimant les deux lignes :

Code :
  1. ip     = (struct ipheader *) malloc(sizeof(struct ipheader));
  2. icmp = (struct icmpheader *) malloc(sizeof(struct icmpheader));


 
Ainsi il n'y a plus de fuite mémoire :jap:

Un peu comme si tu as un bobo sur la main et que le chirurgien te coupe le bras. Plus de main => plus de bobo  :D  
 

XK a écrit :

Mais est-ce que c'était important d'allouer la mémoire à cet endroit? :??:

Oui !
 
Ce qu'il faut que tu fasses, c'est ajouter les free() correspondants comme dit plus haut dans le topic. Pour le pointeur icmp, qui est modifié après le retour de malloc (problème soulevé par Elmoricq), il faut que tu sauvegardes sa valeur juste après le retour de malloc, et que tu fasses ton free() sur le pointeur sauvegardé.
 
[edit] En lisant en diagonale, j'avais pas vu que les espaces alloués étaient pas utilisés :??:


Message édité par franceso le 05-01-2007 à 11:08:14

---------------
TriScale innov
Reply

Marsh Posté le 05-01-2007 à 11:11:56    

Effectivement, je n'ai pas écris ce code qui reste assez abstrait pour moi, mais il semble que l'espace alloué n'est pas utilisé donc inutile :D
 
Si vous trouvez d'autres choses absurdes ça m'interesse ;)

Reply

Marsh Posté le 05-01-2007 à 13:34:32    

pourquoi tu fais tous ces allocations dynamiques ? M'est d'avis que tu débute et que tu fais un mélange pointeur / malloc.
 
Vire tes malloc.

Reply

Marsh Posté le 05-01-2007 à 13:34:32   

Reply

Marsh Posté le 12-01-2007 à 11:25:07    

J'ai tout simplifié, les malloc sont bien libérés désormais.
 
Par contre j'ai essayé de changer la fonction recv() en recvfrom() :

Code :
  1. Avant :
  2. ret = recv(sock,buffer,sizeof(struct ipheader)+sizeof(struct icmpheader),0);
  4. Maintenant :
  5. ret = recvfrom(sock,buffer,sizeof(struct ipheader)+sizeof(struct icmpheader),0,(struct sockaddr *)&peer,sizeof(struct  sockaddr));


Mais la fonction me renvoi toujours "-1" alors que recv me renvoyait bien la taille du paquet.
Une idée d'où se trouve mon erreur? :hello:
Merci

Message cité 2 fois
Reply

Marsh Posté le 12-01-2007 à 12:26:17    

XK a écrit :

Par contre j'ai essayé de changer la fonction recv() en recvfrom() :

Code :
  1. Avant :
  2. ret = recv(sock,buffer,sizeof(struct ipheader)+sizeof(struct icmpheader),0);
  4. Maintenant :
  5. ret = recvfrom(sock,buffer,sizeof(struct ipheader)+sizeof(struct icmpheader),0,(struct sockaddr *)&peer,sizeof(struct  sockaddr));


Mais la fonction me renvoi toujours "-1" alors que recv me renvoyait bien la taille du paquet.
Une idée d'où se trouve mon erreur? :hello:
Merci


On ne remplace pas recv() par recvfrom() au gré des vents ou de l'humeur du jour... La première fonction est pour le mode connecté (Paquets TCP, par exemple), la deuxième est pour le mode non connecté (Datagrams UDP, par exemple)


---------------
Des infos sur la programmation et le langage C: http://www.bien-programmer.fr Pas de Wi-Fi à la maison : http://www.cpl-france.org/
Reply

Marsh Posté le 12-01-2007 à 12:32:31    

XK a écrit :

Par contre j'ai essayé de changer la fonction recv() en recvfrom() :

Code :
  1. Avant :
  2. ret = recv(sock,buffer,sizeof(struct ipheader)+sizeof(struct icmpheader),0);
  4. Maintenant :
  5. ret = recvfrom(sock,buffer,sizeof(struct ipheader)+sizeof(struct icmpheader),0,(struct sockaddr *)&peer,sizeof(struct  sockaddr));


Mais la fonction me renvoi toujours "-1" alors que recv me renvoyait bien la taille du paquet.
Une idée d'où se trouve mon erreur? :hello:
Merci


Hum... j'ai la bizarre sensation que tu utilises ces fonctions comme une recette magique. On remplace ceci par cela un peu au hasard et on regarde...
Chaque fonction a une utilité précise et c'est quand on connait les fonctions qu'on peut arriver à trouver laquelle sera le plus utile pour résoudre le problème.
En l'occurrence, "recv()" est une fonction destinée à recevoir des infos d'une socket ouverte en prococole TCP, c'est à dire avec le canal source/destinataire déjà créé et le chemin bien établi. En revanche, la fonction "recvfrom()" est une fonction destinée à recevoir des infos d'une socket ouverte en protocle UDP, c'est à dire sans chemin établi. C'est la fonction qui se charge d'établir le chemin à la demande. Et c'est aussi pour ça que 2 appels simultanés à "recvfrom()" peuvent voir le 2° résultat arriver avant le 1er parce que le 2° chemin établi aura été plus court que le premier.
En tout cas on ne peut pas interchanger les 2 fonctions...
 
Commence un peu par te familiariser avec les bases du C, puis t'arrivera au malloc/free puis ensuite tu pourras attaquer les comm. réseau avec les sockets. Mais entre temps t'auras pas mal de trucs à voir...
 
[edit] Toasted  :(


Message édité par Sve@r le 12-01-2007 à 12:33:30

---------------
Vous ne pouvez pas apporter la prospérité au pauvre en la retirant au riche.
Reply

Marsh Posté le 12-01-2007 à 14:05:28    

J'aurais pensé que c'était mieux de spécifier la source du message à recevoir, bien entendu ici nous sommes en mode connecté donc ça n'a que peu d'importance...
Sur le net on trouve parfois n'importe quoi, entre autre conseiller d'utiliser une fonction plutôt qu'une autre alors qu'on en a pas l'interrêt... Donc je resterai avec recv() qui fonctionne bien et je le saurai pour la prochaine fois, merci ;)

Message cité 1 fois
Reply

Marsh Posté le 12-01-2007 à 17:40:53    

XK a écrit :

bien entendu ici nous sommes en mode connecté


Oui, j'avais remarqué

XK a écrit :

donc ça n'a que peu d'importance...


Au contraire, cela en a énormément en ce sens qu'on ne peut pas utiliser "recvfrom()". Ce n'est pas que t'as le choix entre les deux et que cela importe peu d'en prendre l'une plutôt que l'autre, c'est que t'as absolument pas le choix et si tu choisis la mauvaise fonction ben rien ne marchera !!!
 
En revanche, tu peux choisir entre "recv()" et "read()". La première est spécifiquement adaptée aux sockets, la seconde est adaptée à toute entrée Unix (donc la socket en fait partie). Bien entendu "recv()" est mieux mais "read()" marche aussi. C'est comme si tu faisais de la copie de chaîne avec "memcpy()" plutôt que "strcpy()". Même si strcpy() est spécialement faite pour ça, memcpy() marche aussi...

Message cité 1 fois
Message édité par Sve@r le 12-01-2007 à 17:49:02

---------------
Vous ne pouvez pas apporter la prospérité au pauvre en la retirant au riche.
Reply

Marsh Posté le 12-01-2007 à 17:59:56    

Sve@r a écrit :

En revanche, tu peux choisir entre "recv()" et "read()". La première est spécifiquement adaptée aux sockets, la seconde est adaptée à toute entrée Unix (donc la socket en fait partie). Bien entendu "recv()" est mieux mais "read()" marche aussi.


Surtout recv() est plus portable que read()...
 


---------------
Des infos sur la programmation et le langage C: http://www.bien-programmer.fr Pas de Wi-Fi à la maison : http://www.cpl-france.org/
Reply

Sujets relatifs:

Leave a Replay

Make sure you enter the(*)required information where indicate.HTML code is not allowed