¿Posible memory leak con este ejemplo de C++?

* 19 dic 2010 19:58

Enlace permanente

Estoy haciendo un juego en C++ con SDL y me acabo de topar con un posible memory leak. Como un ejemplo vale más que mil palabras... os dejo un sencillo ejemplo de algo muy parecido a lo que tengo y lo que creo que podría tener para mejorarlo

#include <iostream>
#include <vector>
#include <stdlib.h>
#include <string.h>

char* BufferMalloc(const char* txt) {
   char* cadena = (char*)malloc(strlen(txt));
   strcpy(cadena, txt);
   return cadena;
}

char* BufferNew(const char* txt) {
   char* cadena = new char[strlen(txt)];
   strcpy(cadena, txt);
   return cadena;
}

char* BufferVec(const char* txt) {
   std::vector<char*> cadena;
   cadena.push_back(new char[strlen(txt)]);
   strcpy(cadena[0], txt);
   return cadena[0];
}

int main(int argc, char* argv[]) {
   std::cout << BufferMalloc("Hola") << std::endl;
   std::cout << BufferNew("Hola") << std::endl;
   std::cout << BufferVec("Hola") << std::endl;
   return 0;
}

Actualmente estoy usando una función muy similar a BufferNew, pero como no hago un delete[] cadena creo que ahí se produce un memory leak. Creo que una posible solución es usar malloc() pero como tampoco llamo luego a un free() me da que estoy en las mismas. Por último se me ha ocurrido meter la cadena en un vector<char*> (porque si no me equivoco, cuando acaba la función el vector es destruido y llama a los destructores de cada objeto de su interior). ¿Cuál veis que es la solución más óptima? Y si alguien me puede comentar un poquito sobre los posibles memory leaks que puedan producir esas funciones le estaré muy agradecido .

Saludos!

19 dic 2010 21:51

Enlace permanente

Estoy haciendo un juego en C++ con SDL y me acabo de topar con un posible memory leak.

Posible? querrás decir seguro al 100%, para toda petición de memoria con malloc/new debes liberarla con free/delete al terminar de usarla ya que c++ no va a hacerlo por ti, y si, los 3 casos que has puesto tienen leaks. Tienes razón a medias con lo del vector, excepto que solo funciona con referencias a objetos directamente, no con apuntadores a estos. Algo que posiblemente si te funcionaria seria el uso de boost::shared_ptr (o tr1::shared_ptr) para no tener que liberar esa memoria por tu cuenta, mírate la documentación de boost/c++0x ya que no que te puedo ayudar mucho con ese tema (no he tenido el tiempo para trabajar con boost como quisiera ).

P.D.: y para depurar esos leaks puedes empezar usando valgrind.

* 20 dic 2010 14:04

Enlace permanente

Muchas gracias por la respuesta . Como bien habías dicho, con Valgrind he comprobado que las 3 funciones tenían leaks y al final he optado por usar punteros y declarar el char* fuera de la función para luego poder liberar la memoria. Más o menos algo así:

#include <iostream>
#include <cstring>

void ToBuff(const char *txt, char **buffer ) {
   (*buffer) = new char[strlen(txt)];
   strcpy((*buffer), txt);
}

int main(int argc, char* argv[]) {
   char *buff;
   ToBuff("Hola", &buff);
   std::cout << buff << std::endl;
   delete[] buff;
   return 0;
}

La verdad es que está muy bien el Valgrind, mea culpa por no haber investigado más sobre programas para debug y detección de errores. Muchas gracias también por la recomendación

Saludos!

20 dic 2010 17:59

Enlace permanente

1) Si usas C++, ¿por qué no usas std::string?
2) Las cadenas delimitadas por " son de tipo 'const char *', no deberías hacer ese cast a char*. Es más, en C++ no deberías usar casts 'tipo C'.
3) sizeof(txt) == sizeof(char*) == 4 en x86. Te funciona de chiripa. Eso está MAL. Realmente quieres usar strlen(txt) y no sizeof.
4) Me pregunto cuál es el objetivo de la función ToBuff.

- ferdy

20 dic 2010 18:33

Enlace permanente

1) Si usas C++, ¿por qué no usas std::string?

Ahí te doy la razón, debería usar std::string y a la hora de pasar una string como argumento añadirle el 'c_str()'. Es un "mal vicio" que se me ha quedado de C.

2) Las cadenas delimitadas por " son de tipo 'const char *', no deberías hacer ese cast a char*. Es más, en C++ no deberías usar casts 'tipo C'.

Ahí te vuelvo a dar la razón, en el código original lo tengo puesto como 'const char*' pero al escribir el ejemplo se me ha pasado y para evitar el error de 'warning: deprecated conversion from string constant to ‘char*’ ' lo primero que me ha venido a la cabeza es usar los cast de C. Me he dado cuenta más tarde pero tampoco lo he visto tan grave como para editar

3) sizeof(txt) == sizeof(char*) == 4 en x86. Te funciona de chiripa. Eso está MAL. Realmente quieres usar strlen(txt) y no sizeof.

Por 3ª vez te doy la razón . No se en qué estaba pensando cuando puse sizeof() en vez de strlen()... realmente funciona de chiripa como dices, voy a editar ya mismo.

4) Me pregunto cuál es el objetivo de la función ToBuff.

Su función es pasarle un paquete de archivos y extraer el archivo solicitado (que devuelve en el buffer) para luego cargar el archivo de ese buffer (que es de audio/imagen) con SDL.

Muchas gracias por los apuntes, Ferdy. A ver si me voy quitando ya las manías que acarreo de C...

Saludos!

20 dic 2010 19:06

Enlace permanente

4) Me pregunto cuál es el objetivo de la función ToBuff.

Su función es pasarle un paquete de archivos y extraer el archivo solicitado (que devuelve en el buffer) para luego cargar el archivo de ese buffer (que es de audio/imagen) con SDL.

Ahhh ok... perdona, la pregunta era estúpida.

Muchas gracias por los apuntes, Ferdy. A ver si me voy quitando ya las manías que acarreo de C...

Suele ocurrir. El mejor consejo es dejar de pensar que C y C++ son lenguajes parecidos

- ferdy