[c++] problema new delete e vector

[czar]

allora, ho del codice preesistente che suppongo crei dei memory leak, volevo dargli una sistemata ma ho trovato qualche problema, questo è il codice:

[...]

				unsigned shape_count = 0;
				vector<double*> pvec, svec, cvec, fmvec, fsdvvec;
				vector<double> avec;

				//for all points of contour
				for (; cont; cont = cont->h_next)
		  		{
[...]
					{
[...]
						double *feature = new double[3];
						double *color = new double[3];
						double *position = new double[2];

						compute_shape_feature(points, shape_num_point, shape_per, shape_area, feature, position, newFeatures, subImageMargin, startP, endP);
						compute_color_feature(imgN, position, color);


[...]

//QUESTA DICHIARAZIONE L'HO MESSA IO, E' CORRETTO COME ALLOCO IL VETTORE E COME LIBERO LA MEMORIA?
				  		GlcmMatrix *directionalGLCM[4];
				  		directionalGLCM[0] = new GlcmMatrix(GlcmLevel, WindowSize, 1, 0);
				  		directionalGLCM[1] = new GlcmMatrix(GlcmLevel, WindowSize, 1, -1);
				  		directionalGLCM[2] = new GlcmMatrix(GlcmLevel, WindowSize, 0, 1);
				  		directionalGLCM[3] = new GlcmMatrix(GlcmLevel, WindowSize, 1, 1);
[...]

				  		//free the memory
						for (int k = 0; k < 4; k++)
						{
							delete directionalGLCM[k];
						}



						#ifdef DEBUG
						cout << endl << "Shape " << shape_count << "------------------" << endl;
						cout << "Contour's points: " << shape_num_point << endl;
						cout << "Perimeter: " << shape_per << endl;
						cout << "Area: " << shape_area << endl;
						cout << "Feature: " << feature[0]  << " " << feature[1] << " " << feature[2] << endl;
						cout << "Color: " << color[0]  << " " << color[1] << " " << color[2] << endl;
						cout << "Center Position: " << position[0] << " " << position[1] << endl;
						#else
						cout << ".";
						pvec.push_back(position);
						cvec.push_back(color);
						svec.push_back(feature);
						avec.push_back(shape_area);
						#endif

						//[red]memory leak!!! SE LO DECOMMENTO MEMORIZZA NEL FILE SOLO L'ULTIMO INSIEME DI VALORI RIPETUTO TANTE VOLTE QUANTE ESEGUE IL CICLO[/red]
						//delete [] feature;
						//delete [] color;
						//delete [] position;

					}
		  		}
[...]

				cout << "ok" << endl;

				vector<double*>::iterator its = svec.begin(), itc = cvec.begin(), itp = pvec.begin();
			    vector<double>::iterator ita = avec.begin();

			    //append to file
			    fprintf(pFile, "%s\n", fname.str().c_str());
		    	fprintf(pFile, "%d\n", shape_count);

			    while(its != svec.end())
			    {
			    	fprintf(pFile,"%f\n", *ita);
			    	fprintf(pFile, "%f %f\n", (*itp)[0], (*itp)[1]);
			    	fprintf(pFile, "%f %f %f\n", (*its)[0], (*its)[1], (*its)[2]);
			    	fprintf(pFile, "%f %f %f\n", (*itc)[0], (*itc)[1], (*itc)[2]);
			    	its++; itc++; itp++; ita++;
			    }

come ho scritto anche nel codice, se provo a fare il delete, tutti i gruppi di valori del file sono uguali, se lascio tutto commentato invece funziona tutto bene (anche se credo crei memory leak, in quanto ogni cosa con new poi ha bisogno del delete...).
Ho provato anche a dichiarare questi:

						double *feature = new double[3];
						double *color = new double[3];
						double *position = new double[2];

come semplici vettori, togliendo quindi i new e i delete:

						double feature[3];
						double color[3];
						double position[2];

ma il problema non cambia.

Ho anche provato a spostare fuori da ciclo sia le dichiarazioni delle variabili (prima dell'ingresso del ciclo) e il loro delete (dopo la fine del ciclo), ma il risultato non cambia.
E' corretto lasciar le cose così?

[Ikon O'Cluster]

Il codice che hai scritto è frammentario... per quanto ne so quelle classi potrebbero contenere campi allocati dinamicamente e non prevedere un distruttore. Io ti consiglio di prestare più attenzione e valutare bene se è davvero necessario l'uso della memoria dinamica. A questo punto una programmazione meticolosa dovrebbe ridurre al minimo i danni. Ad ogni modo uno strumento come "valgrind" dovrebbe darti una mano ad individuare la magagna!

[tomminno]

Solo una domanda perchè non usi vector al posto di gestire manualmente gli array?

Per chiarire i tuoi dubbi: hai usato vector<double*>, se fai il delete degli array poi chiaramente non hai più i valori a disposizione nel vector. Devi deallocare tutti gli elementi solo dopo averli usati.
Ti consiglierei di usare vector<double> al posto di vector<double*>

[czar]

Originariamente inviato da Ikon O'Cluster

Il codice che hai scritto è frammentario... per quanto ne so quelle classi potrebbero contenere campi allocati dinamicamente e non prevedere un distruttore. Io ti consiglio di prestare più attenzione e valutare bene se è davvero necessario l'uso della memoria dinamica. A questo punto una programmazione meticolosa dovrebbe ridurre al minimo i danni. Ad ogni modo uno strumento come "valgrind" dovrebbe darti una mano ad individuare la magagna!

no le classi non hanno campi allocati dinamicamente

[czar]

Originariamente inviato da tomminno

Solo una domanda perchè non usi vector al posto di gestire manualmente gli array?

Per chiarire i tuoi dubbi: hai usato vector<double*>, se fai il delete degli array poi chiaramente non hai più i valori a disposizione nel vector. Devi deallocare tutti gli elementi solo dopo averli usati.
Ti consiglierei di usare vector<double> al posto di vector<double*>

non ho scritto io quella parte di codice, ma potrei cambiarlo, chi l'ha scritto penso l'abbia scritto così in modo da poter fare il push di un vettore intero, senza farlo elemento per elemento.

Ma lasciandolo così, con solo quello che vedete dato che il resto è apposto, crea memory leak non deallocare quei tre vettori double, o va bene?

[tomminno]

Originariamente inviato da czar

non ho scritto io quella parte di codice, ma potrei cambiarlo, chi l'ha scritto penso l'abbia scritto così in modo da poter fare il push di un vettore intero, senza farlo elemento per elemento.

Sarebbe?
Dal codice si vede che quegli array sono valorizzati da compute_shape_feature, usare un vector è molto più comodo.

Ma lasciandolo così, con solo quello che vedete dato che il resto è apposto, crea memory leak non deallocare quei tre vettori double, o va bene?

Certo che crea leak, ma li devi deallocare dopo averli usati, per come hai messo te le delete li cancelli prima di usarli.
Chiaro che così non funziona.

[czar]

credo di aver capito.
Ma considerando il fatto che il file viene scritto alla fine del programma e che poi questo termini, SE alla chiusura del programma tutta la memoria che occupava viene liberata, credo sia inutile fare il delete (questo però avviene alla chiusura del programma?)

Un altro consiglio, mi conviene spostare all'interno del primo for la scrittura del file in modo da avere cicli in meno?
Io suppongo di si.

[Ikon O'Cluster]

Originariamente inviato da czar

credo di aver capito.
Ma considerando il fatto che il file viene scritto alla fine del programma e che poi questo termini, SE alla chiusura del programma tutta la memoria che occupava viene liberata, credo sia inutile fare il delete (questo però avviene alla chiusura del programma?)

Tu fai sempre la delete... aspettare la chiusura del programma non è una soluzione saggia. Se tu stessi realizzando un server o una applicazione con tempi di esecuzione lunghi, rischieresti di sprecare molta memoria.

[czar]

Originariamente inviato da Ikon O'Cluster

Tu fai sempre la delete... aspettare la chiusura del programma non è una soluzione saggia. Se tu stessi realizzando un server o una applicazione con tempi di esecuzione lunghi, rischieresti di sprecare molta memoria.

e spostando la scrittura del file dentro il ciclo for, risparmierei tempo rispetto a com'è adesso che uso un while per scrivere il file, giusto?

[Ikon O'Cluster]

Sinceramente non ho capito! Adesso cicli N volte il for ed M volte il while. Ottieni N*M scritture del file. Ovvio che se lo metti fuori dal while ne fai solo N, ma devi vedere se puoi farlo. Se quel while esiste ci sarà un motivo no?

[czar]

Originariamente inviato da Ikon O'Cluster

Sinceramente non ho capito! Adesso cicli N volte il for ed M volte il while. Ottieni N*M scritture del file. Ovvio che se lo metti fuori dal while ne fai solo N, ma devi vedere se puoi farlo. Se quel while esiste ci sarà un motivo no?

si in effetti il motivo della sua esistenza c'è, in pratica dentro il for (eseguito diciamo N volte) c'è un if che non ho scritto e se un valore è minore di una soglia allora va in "continue". Altrimenti si fa il blocco di codice che ho scritto e incrementa un contatore (shape_count, che alla fine sarà pari alle M volte del while) che ho omesso per evitarvi di annoiarvi a leggere tutto il codice.
In pratica quel while esiste per scrivere in testa agli elementi del file, il numero totale di shape_count.
Pensavo quindi di fare il conto degli shape_count prima, in modo da non dovermi trascinare troppe variabili da deletare, e scrivere il file direttamente nel ciclo for, invece che fuori.

[Ikon O'Cluster]

Se il tuo problema è ottimizzare le vie in linea di principio sono:

1) Evitare il più possibile operazioni di I/O
2) Evitare il più possibile allocazione dinamica
3) Cercare di ridurre il più possibile le operazioni all'interno dei cicli

[czar]

perfetto grazie

un ultima cosa, questa allocazione/deallocazione è corretta vero? (nell'oggetto non ci sono tipi dinamici)

				  		GlcmMatrix *directionalGLCM[4];
				  		directionalGLCM[0] = new GlcmMatrix(GlcmLevel, WindowSize, 1, 0);
				  		directionalGLCM[1] = new GlcmMatrix(GlcmLevel, WindowSize, 1, -1);
				  		directionalGLCM[2] = new GlcmMatrix(GlcmLevel, WindowSize, 0, 1);
				  		directionalGLCM[3] = new GlcmMatrix(GlcmLevel, WindowSize, 1, 1);
[...]

				  		//free the memory
						for (int k = 0; k < 4; k++)
						{
							delete directionalGLCM[k];
						}

[Ikon O'Cluster]

Si è corretto...