[C] Errori sulla free() aiuto!!!!!!!!

[Manugal]

Ciao

Ho compilato il progetto che devo consegnare per l'Università. Nelle specifiche mi si chiede che quando viene rilevato un certo tipo di errore devo liberare tutta la memoria allocata. Vi posto una parte del mio codice:

int CreateDatabaseFile(char *Name, int NumFields, FIELD_DATA_t *FieldData){
    int i;
    char *finale;
    header *h;
    Name=(char *)malloc(sizeof(char)*DBF_NAME_LENGTH);
    h=(header *)malloc(sizeof(header)*(DBF_FIXED_HEADER_LEN+MAX_NUM_FIELDS*(DBF_FIELD_LEN+MAX_NUM_FIELDS)+2));
    FieldData=(FIELD_DATA_t *)malloc(sizeof(FIELD_DATA_t)*MAX_NUM_FIELDS);
    if((Name==NULL) || (cercaErr(Name)==1)){
       char Err1[256];
       (void) sprintf(Err1, "Nome DB nullo o non valido o NumFields<=0");
       LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err1);
       free(Name);
       free(FieldData);
       free(h);
       return LPC_BAD_ARG;
    }
    
    if((NumFields<=0) || (NumFields>MAX_NUM_FIELDS)){
       char Err10[256];
       (void) sprintf(Err10, "Nome DB nullo o non valido o NumFields<=0");
       LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err10);
       free(Name);
       free(FieldData);
       free(h);
       return LPC_BAD_ARG;
    }
...........
...........
...........

Quando vado ad eseguire, ad esempio il primo test, che verifica se Name==NULL, mi appare un messaggio del genere:

*** glibc detected *** double free or corruption (fasttop): 0x08053008 ***

Perché fa così? Io mi sono attenuto strettamente alle specifiche che mi sono state date (e le free() vanno messe lì).

[sottovento]

Ciao,
se Name== NULL, quindi non hai allocato memoria, tenti di deallocarla. Ovviamente avrai l'errore indicato.

High Flying
Sottovento

[Manugal]

Ma Name io l'ho allocata poco prima, quello è solo un controllo.

[Manugal]

Cmq ho provato a toglierla ma mi dà lo stesso errore

[sottovento]

Ciao,
nel codice da te pubblicato, ho visto che allochi memoria per diverse variabili. Una volta allocata, dovresti controllare che l'allocazione sia andata a buon fine, cioe' che il puntatore abbia valore diverso da NULL.

Anche se hai tolto quella free(), ne hai lasciate altre che vanno a deallocare, pur non sapendo se l'allocazione precedente e' andata a buon fine.

Per esempio, fai una
free (h);

se h == NULL hai ancora lo stesso problema.
Ti ricordo che l'allocazione potrebbe fallire non solo quando chiedi un quantitativo di memoria non disponibile, ma anche se i parametri sono errati e se richiedi di allocare 0 byte.

Per tagliare la testa al toro, prima di deallocare potresti fare

if (h != NULL)
free (h);

Cmq DEVI mettere il controllo sull'esito dell'allocazione. E' imperativo!

High Flying
Sottovento

[Manugal]

Ah quindi tu dici di fare una cosa del genere appena va incontro ad un errore:

if((Name==NULL) || (cercaErr(Name)==1)){
       char Err1[256];
       (void) sprintf(Err1, "Nome DB nullo o non valido o NumFields<=0");
       LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err1);
       free(Name);
       if(FieldData!=NULL)
          free(FieldData);
       if(h!=NULL)
          free(h);
       return LPC_BAD_ARG;
    }

Ora provo e ti faccio sapere Grazie

[Manugal]

Niente da fare sempre lo stesso messaggio

Posto il codice con le modifiche:

int CreateDatabaseFile(char *Name, int NumFields, FIELD_DATA_t *FieldData){
    int i;
    char *finale;
    header *h;
    Name=(char *)malloc(sizeof(char)*DBF_NAME_LENGTH);
    h=(header *)malloc(sizeof(header)*(DBF_FIXED_HEADER_LEN+MAX_NUM_FIELDS*(DBF_FIELD_LEN+MAX_NUM_FIELDS)+2));
    FieldData=(FIELD_DATA_t *)malloc(sizeof(FIELD_DATA_t)*MAX_NUM_FIELDS);
    if((Name==NULL) || (cercaErr(Name)==1)){
       char Err1[256];
       (void) sprintf(Err1, "Nome DB nullo o non valido o NumFields<=0");
       LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err1);
       if(h!=NULL)
          free(h);
       if(FieldData!=NULL)
          free(FieldData);
       return LPC_BAD_ARG;
    }
    
    if((NumFields<=0) || (NumFields>MAX_NUM_FIELDS)){
       char Err10[256];
       (void) sprintf(Err10, "Nome DB nullo o non valido o NumFields<=0");
       LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err10);
       if(Name!=NULL)
          free(Name);
       if(h!=NULL)
          free(h);
       if(FieldData!=NULL)
          free(FieldData);
       return LPC_BAD_ARG;
    }
    if(FieldData==NULL){
       char Err4[256];
       (void) sprintf(Err4, "Errore allocazione memoria");
       LPC_GestioneErrore(LPC_NO_MEMORY, "CreateDatabaseFile", Err4);
       if(Name!=NULL)
          free(Name);
       if(h!=NULL)
          free(h);
       return LPC_NO_MEMORY;
....
....
....

[Manugal]

Qualcuno mi aiuta? Sto uscendo quasi pazzo

[heroth]

Sei sicuro di non eseguire più di una volta la free()?

[Manugal]

In che senso scusa..

La free() l'ho scritta come la vedi anche tu.

[wingman87]

Le ultime 4 righe degli if sono uguali, prova a metterle fuori dagli if alla fine, così non può dire che l'hai fatto 2 volte...

[heroth]

nel senso che magari hai più condizioni di errore vere (if) e pertanto iteri la free sempre sullo stesso puntatore. Potresti effettuare la free e dopo mettere a null il puntatore appena liberato per vedere o se funziona o se hai un nuovo errore... da lì dovresti dedurne qualcosa

[Qu@ker]

jcd@big:~$ man 3 free | sed -ne '23,28p'

       free() frees the memory space pointed to by ptr, which must  have  been
       returned by a previous call to malloc(), calloc() or realloc().  Other-
       wise, or if free(ptr) has already been called before, undefined  behav-
       iour occurs.  If ptr is NULL, no operation is performed.

jcd@big:~$

[vlacus]

hai provato con mtrace?

[Manugal]

Eccomi

Allora nelle specifiche che mi hanno dato mi viene chiesto espressamente di liberare tutta la memoria allocata in quei punti dove si gestiscono gli errori. Però non ho capito cosa dovrei provare (sono un po' di coccio ). Cioè devo fare la free nella gestione dell'errore e poi controllo se quello che ho appena liberato è NULL? Cmq se può esservi utile vi posto la funzione che esegue i test.

/*!
*	\addtogroup Modulo1
*	\{
*/
/*!
*	\addtogroup TestModulo1 Specifiche per il Test
*	\{
*/

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include "LPC_M1_Include.h"


void TestCreate(void);	//!< Funzione di Test per la CreateDatabaseFile()
void TestDelete(void);	//!< Funzione di Test per la DeleteDatabaseFile()
void Result(char *Func);	//!< Funzione di calcolo e stampa risultati



// Vettori per gli id dei test, il check e i pesi relativi
#define MAX_TEST 100
typedef struct {
	int testid;
	int weight;
	char desc[256];
	int passed;
} TEST_t;
		
static int numtest;
static TEST_t test[MAX_TEST];

/*!
*	\brief Test per le funzioni di interfaccia del Modulo 1
*
* Programma di test per il Modulo 1
*
* Ogni test e' identificato da un numero cosi' composto:\n
* \p xyynn \n
*  x	Id del modulo\n
* yy	Id della funzione\n
* nn	Id del test\n
*
* Ogni test ha anche un peso che viene indicato nella stampa dei risultati.
* I test di funzionalit�hanno i pesi piu' elevati.
*
* Tramite menu possono essere richiamate sia le singole funzioni di test TestCreate() e TestDelete(),
* che un test completo con un punteggio finale.
*
* Vengono effettuati i seguenti controlli: 
* - Creazione (CreateDatabaseFile()): 
*   - Nome del DBF errato (NULL o caratteri non validi o inizio non valido)
*   - Numero campi non valido
*   - FieldData non valido
*   - Campo non valido (Nome, Tipo o Lunghezza o gia' esistente)
*   - File gia' esistente
*   - Verifica funzionalita'
*   - Verifica creazione corretta (confronto binario con template)
*
* - Cancellazione (DeleteDatabaseFile()):
*   - Nome del DBF errato (NULL o caratteri non validi o inizio non valido)
*   - DBF non esistente
*   - DBF aperto
*   - Verifica funzionalita'
* 
****************************************************/


int main(int argc, char **argv)
{
	char scelta[10];
	int sel;

	printf("Programma di Test per il modulo 1\n\n");


	for(;;)
	{
		printf("\t1. Test funzione CreateDatabase\n");
		printf("\t2. Test funzione DeleteDatabase\n");
		printf("\t3. Test completo e valutazione\n");
		printf("\n\t0. Uscita\n");
		printf("\n\nScelta: ");

		(void) fgets(scelta, sizeof(scelta), stdin);
		sel = atoi(scelta);
		Result(NULL);
		switch(sel)
		{
		case 0:
			printf("Bye!\n\n");
			return(0);
		case 1:
			TestCreate();
			break;
		case 2:
			TestDelete();
			break;
		case 3:
			TestCreate();
			TestDelete();
			break;
		default:
			printf("Scelta errata...\n");
			break;;
		}
	}
}

/*********************************
* \brief Richiama le funzioni per il test della CreateDatabaseFile()
**********************************/

void TestCreate(void)
{
	int ret= 0;
	int NumFields = 0;
	FILE *fp;
	FIELD_DATA_t FieldData[MAX_NUM_FIELDS];
	int len, len1;
	char buf[500], buftest[500];

	printf("Test CREATE\n");
    
// Test sul nome
	// null
	test[numtest].testid = 10101;
	test[numtest].weight = 3;
	strcpy(test[numtest].desc, "CREATE: nome NULL");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	ret = CreateDatabaseFile(NULL, NumFields, &FieldData[0]);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

	// Non valido
	test[numtest].testid = 10102;
	test[numtest].weight = 1;
	strcpy(test[numtest].desc, "CREATE: nome non valido");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	ret = CreateDatabaseFile("PIppo-1", NumFields, &FieldData[0]);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

	test[numtest].testid = 10103;
	test[numtest].weight = 1;
	strcpy(test[numtest].desc, "CREATE [%d]: nome non valido (2)");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	ret = CreateDatabaseFile("_pippo", NumFields, &FieldData[0]);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;


// NumFields
	test[numtest].testid = 10104;
	test[numtest].weight = 3;
	strcpy(test[numtest].desc, "CREATE: NumFields nonvalido");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	ret = CreateDatabaseFile("Prova", 0, &FieldData[0]);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

	test[numtest].testid = 10105;
	test[numtest].weight = 3;
	strcpy(test[numtest].desc, "CREATE: NumFields nonvalido (2)");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	ret = CreateDatabaseFile("Prova", MAX_NUM_FIELDS+1, &FieldData[0]);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

// FieldData
	test[numtest].testid = 10106;
	test[numtest].weight = 3;
	strcpy(test[numtest].desc, "CREATE: FieldData nonvalido");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	ret = CreateDatabaseFile("Prova", 5, NULL);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;


// Campo non valido 
/*** Test del Nome, Tipo, Lunghezza *****/
	test[numtest].testid = 10107;
	test[numtest].weight = 1;
	strcpy(test[numtest].desc, "CREATE: Nome campo non valido");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	NumFields = 1;
	strcpy(FieldData[0].Name, "_Campo1");
	FieldData[0].FieldType = CHARACTER;
	FieldData[0].FieldLen = 10;
	ret = CreateDatabaseFile("Prova", NumFields, FieldData);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

	test[numtest].testid = 10108;
	test[numtest].weight = 3;
	strcpy(test[numtest].desc, "CREATE: Tipo campo non valido");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	NumFields = 1;
	strcpy(FieldData[0].Name, "Campo1");
	FieldData[0].FieldType = 15;
	FieldData[0].FieldLen = 10;
	ret = CreateDatabaseFile("Prova", NumFields, FieldData);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

	test[numtest].testid = 10109;
	test[numtest].weight = 2;
	strcpy(test[numtest].desc, "CREATE: Lunghezza campo non valida");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	NumFields = 1;
	strcpy(FieldData[0].Name, "Campo1");
	FieldData[0].FieldType = CHARACTER;
	FieldData[0].FieldLen = FIELD_CHAR_LEN+1;
	ret = CreateDatabaseFile("Prova", NumFields, FieldData);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

	test[numtest].testid = 10110;
	test[numtest].weight = 2;
	strcpy(test[numtest].desc, "CREATE: Campo gia' esistente");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	NumFields = 2;
	strcpy(FieldData[0].Name, "Campo1");
	FieldData[0].FieldType = CHARACTER;
	FieldData[0].FieldLen = 10;
	strcpy(FieldData[1].Name, "Campo1");
	FieldData[1].FieldType = CHARACTER;
	FieldData[1].FieldLen = 10;
	ret = CreateDatabaseFile("Prova", NumFields, FieldData);
	printf("Ritorno della create: [%d]\n\n", ret);
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

// File esistente
	test[numtest].testid = 10111;
	test[numtest].weight = 4;
	strcpy(test[numtest].desc, "CREATE: DBF esistente");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	fp = fopen("Prova.dbf", "w");
	fclose(fp);
	NumFields = 1;
	strcpy(FieldData[0].Name, "Campo1");
	FieldData[0].FieldType = CHARACTER;
	FieldData[0].FieldLen = 10;
	ret = CreateDatabaseFile("Prova", NumFields, FieldData);
	printf("Ritorno della create: [%d]\n\n", ret);
	remove("Prova.dbf");
	if (ret != LPC_OK)
		test[numtest].passed = 1;
	numtest++;

// Funzionalita'
	NumFields = 6;
	strcpy(FieldData[0].Name, "Campo1");
	FieldData[0].FieldType = CHARACTER;
	FieldData[0].FieldLen = 10;

	strcpy(FieldData[1].Name, "Campo2");
	FieldData[1].FieldType = NUMERIC;

	strcpy(FieldData[2].Name, "Campo3");
	FieldData[2].FieldType = DATE;

	strcpy(FieldData[3].Name, "Campo4");
	FieldData[3].FieldType = LOGICAL;

	strcpy(FieldData[4].Name, "Campo5");
	FieldData[4].FieldType = CHARACTER;
	FieldData[4].FieldLen = 1;

	strcpy(FieldData[5].Name, "Campo6");
	FieldData[5].FieldType = NUMERIC;

	test[numtest].testid = 10112;
	test[numtest].weight = 10;
	strcpy(test[numtest].desc, "CREATE: Funzionalita'");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	ret = CreateDatabaseFile("Prova", NumFields, &FieldData[0]);
	printf("Ritorno della create: [%d]\n", ret);
	if (ret == LPC_OK)
		test[numtest].passed = 1;
	numtest++;

// Verifica correttezza
	test[numtest].testid = 10113;
	test[numtest].weight = 10;
	strcpy(test[numtest].desc, "CREATE: Verifica correttezza");
	printf("Test:[%d] Peso:[%d] - [%s]\n", test[numtest].testid, test[numtest].weight,test[numtest].desc);
	for(;;)
	{
		ret = -1;

		fp = fopen("ProvaTest.dbf", "r");
		len = fread(buftest, sizeof(char), 500, fp);
		fclose(fp);

		fp = fopen("Prova.dbf", "r");
		len1 = fread(buf, sizeof(char), 500, fp);
		fclose(fp);
		if (len1 != len)
			break;

		// Confronto la prima parte (prima del timestamp)
		len = DBF_NAME_LENGTH+1+2+4+2+2;
		if (memcmp(buf, buftest, len))
			break;

		// Confronto la seconda parte (dopo il timestamp)
		if (memcmp(buf+len+4, buftest+len+4, len1-len-4))
			break;

		ret = LPC_OK;
		break;
	}
	if (ret == LPC_OK)
		test[numtest].passed = 1;
	numtest++;

	Result("CREATE");

}

[Manugal]

Per mtrace devo digitare mtrace <file>.c? Perché così mi dà No memory leaks.

[mr_hyde]

Ci sono un po' di cose che hai fatto che non ho capito benissimo:

1) perche' l'allocazione di Name? perche' l'allocazione di FieldData?
Da quello che ho capito devono solo essere in input!
2) perche' per segnalare l'errore tutte le volte ti dichiari un buffer _locale_ sempre diverso (Err1, Err10)?
Sei sicuro che LPC_GestioneErrore richieda quell'array locale?

Comunque ho buttato giu' la seg. traccia, forse ti puo' essere utile.
Ricorda che vuole solo essere una traccia, probabilmente neanche compila.

Un consiglio: quando fai una funzione, COME PRIMA COSA controlla i parametri che ti vengono passati.
Solo dopo che sei certo della correttezza dei parametri mettiti a fare il codice per la loro elaborazione.

Ciao,
Mr Hyde

int CreateDatabaseFile(const char *Name, int NumFields, FIELD_DATA_t *FieldData)
{

	header *h      = NULL;
	char   Err[256];

	memset((void*) Err, 0x0, 256);

	/* prima e' meglio controllare questo senza disturbare le allocazioni */
	if ((NumFields<=0) || (NumFields>MAX_NUM_FIELDS))
	{
		snprintf(Err, 256, "Nome DB nullo o non valido o NumFields<=0");
		LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err);
		return LPC_BAD_ARG;
	}

	if (!Name)
	{
		snprintf(Err, 256, "Parametro Name NULL ");
		LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err);
		return LPC_BAD_ARG;
	}

	if (!FieldData)
	{
		snprintf(Err, 256, "Array FieldData non valido");
		LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err);
		return LPC_BAD_ARG;

	}

	if (cercaErr(Name)==1)
	{
		snprintf(Err, 256, "Parametro Name invalido ");
		LPC_GestioneErrore(LPC_BAD_ARG, "CreateDatabaseFile", Err);
		return LPC_BAD_ARG;
	}

	h = (header *) malloc(sizeof(header)*(DBF_FIXED_HEADER_LEN+MAX_NUM_FIELDS*(DBF_FIELD_LEN+MAX_NUM_FIELDS)+2));
	if (!h)
	{
		snprintf(Err, 256, "Errore allocazione header");
		LPC_GestioneErrore(LPC_BAD_ALLOC, "CreateDatabaseFile", Err);
		return LPC_BAD_ALLOC;
	}
 
       [...]
}

[ilsensine]

La mia opinione è che il doppio free viene fatto da una libreria di runtime che utilizzi. Potrebbe non essere un errore tuo, quindi; mi sono capitati casi simili.
L'errore ti capita durante l'esecuzione del programma o alla sua chiusura?

[Manugal]

L'errore mi capita in fase di esecuzione dei test. Cmq avevo allocato anche Name e FieldData perché senza allocarli mi dava un segmentation fault. Per quanto riguarda i buffer degli errori si devo allocarli per ogni gestione dell'errore (magari è sufficiente anche allocarne solo 1 invece di fare da Err1 a Err11, questo sì).

Ora che ho provato a togliere l'allocazione di FieldData e Name mi da questo errore (sempre in esecuzione):

*** glibc detected *** free(): invalid pointer: 0xbfea31fa ***

x MrHyde:

hai scritto il codice esattamente come l'avevo scritto io senza le free(). Senza le free funziona però sorgono due problemi:

1 - Le free() sono richieste dalle specifiche
2 - Se non metto le free (credo dipendano da loro) in alcuni test in cui mi deve ritornare un certo errore, me ne ritorna un altro (ad esempio, nel test che verifica se il file del database esiste, mi ritorna un altro errore che non c'entra niente, mentre io gli ho scritto esplicitamente di tornare l'errore associato a quell'evento)

[ilsensine]

Originariamente inviato da Manugal

*** glibc detected *** free(): invalid pointer: 0xbfea31fa ***

Quell'indirizzo cade all'interno dello stack, quindi ovviamente non è valido per la free.