2010-02-02 20 views
7

tengo una estructura malloc() 'd, y después de usarlos, quiero liberar(), pero mi programa se bloquea aquí. ¿Alguien puede decirme qué estoy haciendo mal?Cómo liberar() un malloc() 'd estructurado correctamente?

Aquí está mi código:

struct data 
{ 
char *filename; 
char *size; 
}; 
//primarypcs is a long type variable 
struct data *primary = (struct data *)malloc(primarypcs * sizeof(struct data)); 
memset(primary, 0, sizeof(struct data *) * primarypcs); 
... 
... 
... 
for (i = 0; i < primarypcs; i++) 
{ 
    free(primary[i].filename); //<----my program freezes here 
    free(primary[i].size);  //<----or here 
} 
free(primary); 

Gracias de antemano!

Kampi

EDIT:

¿Cómo puedo memoria correctamente malloc para el nombre de archivo y el tamaño?

Edit2:

Lo siento, pero yo estaba en un apuro, y que no te lo dije toda la información que necesita. Déjame hacerlo ahora :) Básicamente, quiero crear una aplicación, que obtiene la lista de archivos de dos unidades/carpetas determinadas y luego las compara. Pensé (y todavía lo hago), que la forma más fácil es almacenar los nombres de los archivos y su tamaño en una estructura como la mencionada anteriormente. Así que tengo que asignar memoria dinámicamente (creo que esto es lo que ellos llaman) para nombre de archivo y tamaño y de corse para la estructura también.

+2

No veo que asigne memoria para 'filename' y' size'. –

+2

Sí, hay un error en tu código: en el malloc usas sizeof (struct data) que parece correcto, pero en memset usas sizeof (struct data *) que es la mitad del tamaño. Esto significa que tiene solo la mitad de la memoria y la última mitad de la memoria se eliminará cuando intente liberarla. La respuesta debe ser gratuita (primaria); y nada más – cmroanirgo

+0

Si esto es C++, debería usar new y delete. – Xorlev

Respuesta

7

No está presentando el código completo, donde muchas cosas pueden salir mal, pero un error ya es obvio. La línea

memset(primary, 0, sizeof(struct data *) * primarypcs); 

no está haciendo lo que usted cree que está haciendo. No está poniendo a cero todo el conjunto debido a un error de tipo en sizeof. Fue más probable supone que es

memset(primary, 0, sizeof(struct data) * primarypcs); 

Nota sin * bajo sizeof. Debido a este error, la mayoría de los punteros en su matriz contienen basura como sus valores iniciales. Si no los configura en algo significativo en el código omitido, sus llamadas al free recibirán argumentos basura y fallarán.

En general, para reducir la posibilidad de tales errores, es mejor evitar mencionar nombres de tipos en su programa, excepto en las declaraciones. Desde su pregunta está etiquetada C++ (a pesar de que sin duda se parece a C), no es posible deshacerse de las conversiones de tipo de malloc, pero por lo demás me gustaría decir que la siguiente se ve mejor

struct data *primary = (struct data *) malloc(primarypcs * sizeof *primary); 
memset(primary, 0, primarypcs * sizeof *primary); 

Y, como una nota al margen, si el código estaba destinado a ser C++, se puede obtener el mismo resultado de una manera mucho más elegante, compacta y portátil

data *primary = new data[primarypcs](); 

por supuesto, en este caso tendrá que desasignar la memoria utilizando la funcionalidad apropiada de C++ en lugar de free.

+0

¡Hola! Gracias por tu ayuda Este fue uno de mis errores, pero después de corregir mi código por su respuesta, mi programa aún se bloqueó. Más tarde me di cuenta de que había escrito mal una de mis variables, y solo asigné memoria para 5 elementos, sin embargo, inserté más de 5 elementos en la estructura. ¡Gracias de nuevo! – kampi

3

¿Cómo se asignan las cadenas en la estructura? Si están asignados estáticamente a las constantes, no los liberes de esa manera, y todo lo que se necesita es free (primary); Liberar algo que no estaba dañado le dará al administrador de heap un ataque al corazón.

Si los apuntadores de cadena están configurados por malloc() o calloc(), esa es la manera correcta.

0

Reemplazar el bucle for en la parte inferior de su código con free (primary); debería funcionar.

0

Esto se debe a que no ha asignado explícitamente memoria para filename y size. Así que tratar de hacer free(primary[i].filename); o free(primary[i].size); invocaría Comportamiento indefinido.

Solo free(primary) es suficiente.

EDITAR:

Esta pregunta ha sido etiquetado C++. Entonces, la forma C++ es usar new en lugar de malloc para los tipos definidos por el usuario.

Para las diferencias entre new y malloc echar un vistazo a this.

En C++, sólo tiene que escribir

data *primary = new data[primarypcs](); //() for value initialization 
+0

el código de ejemplo se puede considerar como correcto, suponiendo que individualmente malloc el nombre del archivo. tener un 'tamaño' malloc 'parece sospechoso con errores aunque un tamaño probablemente sería mejor (a menos que realmente sea una cadena de caracteres) – cmroanirgo

0

Esto soluciona el problema en memset, lo que causó todo tipo de problemas para tú.

memset(primary, 0, sizeof(struct data) * primarypcs); 

En resumen, dejó la memoria no inicializada al final de su estructura 'primaria'.

2

Si usted está haciendo esto en C++, que (casi con seguridad) no se debe usar algo como:

data *primary = new data[primarypcs](); 

su lugar, debe usar algo como:

struct data { 
    std::string filename; 
    std::string size; 
}; 

std::vector<data> primary(primarypcs); 

En este caso, normalmente puede manejar la administración de la memoria mucho más simple: defina el vector en el alcance donde se necesita, y cuando se salga del alcance, la memoria se liberará automáticamente.

El uso de una matriz nueva (como new x[y]) en C++ es algo de lo que es mejor que esté. Érase una vez (hace 15 años más o menos) era casi la única herramienta disponible, por lo que su uso (a regañadientes) era casi inevitable, pero ese día ya pasó, y han pasado 10 años desde que realmente hubo un buen razón para usarlo

Dado que inevitablemente hay un comentario sobre "excepto en la implementación de algo así como vector", señalaré que, no, incluso cuando está implementando vector, no usa array nuevo - usted (indirectamente, a través de asignador) use ::operator new para asignar memoria sin formato, colocación nueva para crear objetos en esa memoria, y llamadas dtor explícitas para destruir objetos.

+0

+1 por recomendar 'std :: vector' –

1

Como otros han dicho, hay dos cosas que obviamente erróneas en el fragmento que has demostrado:

  1. no asigna memoria para filename y size miembros de las estructuras simplemente asignados,
  2. Su memset() llamada utiliza el tamaño incorrecto.

Su llamada memset() podría simplificarse y corregido por:

memset(primary, 0, primarypcs * sizeof *primary); 

Hay otro problema sutil con su código: C estándar no garantiza que todos los bits de cero es la constante de puntero nulo (es decir, NULL), por lo que memset() no es la forma correcta de establecer un puntero a NULL. Una manera portátil de hacer lo que quieres hacer es:

size_t i; 
for (i=0; i < primarypcs; ++i) { 
    primary[i].filename = NULL; 
    primary[i].size = NULL; 
} 

asignar memoria para filename y size, depende de lo que desea. Supongamos que determina que filename necesita n bytes y size necesita m. A continuación, el bucle cambia a algo como esto:

size_t i; 
for (i=0; i < primarypcs; ++i) { 
    size_t n, m; 
    /* get the values of n and m */ 
    primary[i].filename = malloc(n * sizeof *primary[i].filename); 
    primary[i].size = malloc(m * sizeof *primary[i].size); 
} 

Se puede omitir la multiplicación con sizeof *primary[i].filename y sizeof *primary[i].size de lo anterior, si usted quiere: C garantiza que sizeof(char) es 1. Me escribió el anterior esté completo y que el caso cuando filename y size cambian tipos.

Además, tenga en cuenta que si filename es una cadena de longitud k, entonces necesita (k+1) bytes para que, a causa de la terminación de 0 (por lo n == k+1 arriba).

Si tuviera que adivinar, ¿quiere size almacenar la longitud del filename correspondiente? Si ese es el caso, size no debe ser un char * sino un size_t. Pero como no sé cómo planea usar filename y size, no estoy seguro.

Asegúrese de comprobar el valor de retorno de malloc(). Devuelve NULL en caso de error. Omití el cheque del código anterior por simplicidad.

Su publicación ha sido etiquetada C++ también, por lo que si está dispuesto a usar C++, también hay disponible una solución de C++.

+0

¡Hola! ¿Cómo debo asignar correctamente la memoria para el nombre de archivo y el tamaño? – kampi

+0

¡Hola! el tamaño es char * para un propósito. Obtengo el tamaño del archivo en mucho tiempo, pero luego tengo que usarlo como un carácter, así que lo convierto de int a char, y luego lo almacena en la estructura. – kampi

0

No está configurando la matriz completa, lo que da como resultado que se libere un puntero a la memoria basura. Utilice calloc en lugar de malloc/memset para evitar este error:

struct data *primary = calloc(primarypcs, sizeof(struct data)); 

Este tanto asigna y borra la memoria. Si desea inicializar todas las entradas struct data también:

for (i = 0; i < primarypcs; ++i) { 
    primary[i].filename = malloc(...); 
    primary[i].size = malloc(...); 
} 

(Usted no describen lo que el nombre de archivo son de tamaño son, por lo que deja la ... por que rellene).