sisoputnfrba / so-commons-library

TADs de uso comun en aplicaciones desarrolladas en C
http://sisoputnfrba.github.io/so-commons-library/
GNU General Public License v3.0
106 stars 175 forks source link

Posible memory leak en bitarray.c #181

Closed ArielRotolo closed 3 months ago

ArielRotolo commented 3 months ago

Hola!

Estaba usando el bitarray para el tp y note que hay un puntero que bitarray_create_with_mode recibe pero bitarray_destroy no libera:

t_bitarray *bitarray_create_with_mode(char *bitarray, size_t size, bit_numbering_t mode){
    t_bitarray *self = malloc(sizeof(t_bitarray));

    self->bitarray = bitarray;
    self->size = size;
    self->mode = mode;

    return self;
}

void bitarray_destroy(t_bitarray *self) {
    free(self->bitarray); //Deberia estar esta linea para que el puntero bitarray pasado al crear t_bitarray sea liberado
    free(self);
}

Incluso ni siquiera parece ser necesario que la variable bitarray se reciba por parámetro ya que con recibir el size se podría simplificar con:

t_bitarray *bitarray_create_with_mode(size_t size, bit_numbering_t mode){
    t_bitarray *self = malloc(sizeof(t_bitarray));

    self->bitarray = calloc(size, sizeof(char)); // Inicializa el bitarray en 0;
    self->size = size;
    self->mode = mode;

    return self;
}

void bitarray_destroy(t_bitarray *self) {
    free(self->bitarray);
    free(self);
}
mgarciaisaia commented 3 months ago

Como regla general, en un TAD no liberás memoria que no reservaste vos, porque no sabés de dónde vino esa memoria (es decir, no tenés garantías de que sea liberable, siquiera). Podés poner como restricción de tu API que vas sa terminar liberando esa memoria (y entonces pasa a ser problema del usuario asegurarse de que la memoria que te pase sea liberable), pero en general nadie gana nada con eso, y limitás casos de uso (no te pueden pasar memoria no-liberable por free). Digamos, si alguien va a llamar a tu xxxx_destroy de todos modos, puede tomarse la molestia de hacer un free justo después del destroy si hiciera falta.

Pero haciendo esto que hace este TAD ganás en eso: podés pasarle un cacho de memoria que no sea manejada por malloc/free. Seguramente en este TAD lo implementamos así para usar en algún TP cuyo bitarray estaba persistido en un archivo (en el filesystem), y entonces podés mmapear ese archivo y usar el TAD de bitarray directamente contra ese archivo mappeado en memoria, y entonces trabajás de manera medio transparente con ese bitarray que está persistido en el disco.

Y si querés laburar con un bitarray en memoria, buen, tampoco es tan complejo: hacés un malloc() antes de llamar al create, y le pasás ese array. Y después lo liberás cuando llamás a destroy.

Y el TAD soporta dos casos de uso distintos por el precio de uno. Una ganga.