Discussion :

[pingouin84k]

Bonsoir à tous,

Je ne comprends pas mon erreur... Comme je débute, je sollicite vos lumières.
J'ai une classe qui contient un tableau déclaré et initialisé comme suit:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
float* _tab;
_tab = new float(n);

Logiquement le destructeur de la classe contient ceci:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

delete[] _tab;

Et pourtant à l'utilisation j'ai une erreur. En effet, à l'utilisation de la classe j'utilise un vecteur (vector) de celle-ci. J'obtiens un crash du programme (la compilation ne pose pas de problème) à partir du deuxième push_back.

Voici un exemple minimaliste qui reproduit mon problème:
.h

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
#pragma once
 
class C01
{
public:
	C01(void);
	~C01(void);
 
protected:
	float* _tab;
	static int _n;
};

.cpp

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
#include "C01.h"
 
int C01::_n = 100;
C01::C01(void) :
_tab(0)
{
	_tab = new float[_n];
}
C01::~C01(void)
{
	delete[] _tab;
}

.cpp

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
#include "C01.h"
#include <vector>
using namespace std;
 
int main()
{
	vector<C01> lst;
	lst.push_back(C01());
	lst.push_back(C01());
	lst.push_back(C01());
 
	return 0;
}

J'utilise VC++ 2010 Express. Voici la fenêtre qui pop.


Par ailleurs, je me suis rendu compte qu'en supprimant le contenu du destructeur, il n'y avait plus de crash. Cependant je ne suis pas sûr que ce soit une bonne chose. J'ai entendu parlé de risque de fuite de mémoire... (je débute)


D'avance, merci pour votre aide!

[foetus]

Parce qu'il te manque le constructeur par recopie

Sinon:

• Tester si sa variable est NULL avant de faire le delete. Même si là, cela ne change rien
• Ton utilisation de ta liste d'initialisation: mets carrément _tab(new float[_n]).
• Ne mets pas void pour le passage de paramètre vide. C'est du C++, pas du C.
• Ne pas nommer avec des tirets bas commençant ou fermants. Même si en théorie ce pseudo-problème est réglé.
• Ne pas utiliser de protected si cela ne sert à rien.

Édit: dans ton premier code tu as mis new float(n) au lieu de new float[n]

[PilloBuenaGente]

Bonjour,

Si la taille du tableau est connue, on peut tout simplement écrire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
class C01
{
public:
 
private:
    float m_tab[100];
};

Ou encore mieux, utiliser un std::array -> std::array<float, 100> tab;Pour ces deux containers l'allocation est gérée automatiquement (pas de new ni delete).

[therwald]

et pour un tableau de taille inconnue, utilises plutôt std::vector<float>, qui sera également géré automatiquement (pas besoin de delete, copie déjà gérée).
Sinon, comme mentionné par foetus ton problème vient de l'absence de constructeur de copie. Le vector va faire des copies lors du push_back. Quand un des tes objets est copié, la copie reçoit une copie du POINTEUR vers le tableau de l'original (comportement de copie par défaut). Quand l'original est détruit le tableau est libéré par le delete du destructeur, mais la copie a toujours son pointeur vers la même zone, qui est désormais invalide. Dès lors, à la première tentative d'utilisation, tu commence les lectures / écritures invalides. Si tu as de la chance ça crashe direct. Sinon, la mémoire a été réallouée, et là ça fait n'importe quoi...
conclusion 1: si tu gères des pointeurs dans ta classe, il faut redéfinir le constructeur de recopie
conclusion 2: moins tu gères de pointeurs, mieux tu te portes

[pingouin84k]

Bonjour à tous et merci de m'avoir répondu.

J'ai essayé d'implémenter un constructeur par copie, mais sans succès (je l'ai peut être mal fait).
Voici ce que j'ai:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
#include "C01.h"
 
// initialisation du nombre d'éléments
int C01::_n = 100;
 
// constructeur par défaut
C01::C01(void) :
_tab(new float[_n])
{
}
 
// constructeur par copie
C01::C01(C01& c01) :
_tab(c01._tab)
{
}
 
// destructeur
C01::~C01(void)
{
	delete[] _tab;
}

Malgré cela j'ai toujours le crash.

Si la taille du tableau est connue, on peut tout simplement écrire :

Code :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
 
class C01
{
public:
 
private:
    float m_tab[100];
};

Ou encore mieux, utiliser un std::array -> std::array<float, 100> tab;Pour ces deux containers l'allocation est gérée automatiquement (pas de new ni delete).

La taille du tableau est connu, mais pour l'utilisation que je veux en faire la déclaration float m_tab[100]; fait sauter la pile. Mon but est de faire un programme de CFD. J'ai l'ambition d'utiliser des tableaux à plusieurs millions d'entrées. On m'a conseillé de déclarer le tableau comme je l'ai fait.


Par ailleurs, je n'utilise pas vector (je n'ai pas essayé array) car en faisant un benchmark je me suis aperçu que les opérations sur un membre déclaré comme suit float* m_tab = new float[n] étaient plus rapides (et de loin) que sur son homologue vector... Visiblement le temps d'accès aux éléments est plus court, mais comme je débute, je ne veux pas trop avancer de certitude.

[therwald]

Pas la bonne signature, ton constructeur de copie:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
C01::C01(const C01& c01) :
_tab(c01._tab)
{
}

du coup en fait tu le le redéfinissait pas, d'où ton problème.

[pingouin84k]

Merci pour ta réponse.
J'ai corrigé mais non ça crash toujours.
On est bien d'accord que dans le destructeur je garde le delete[] _tab;?

[lautrec1]

On est bien d'accord que dans le destructeur je garde le delete[] _tab;?

Et si tu t'en assurais toi-même en faisant le simple test avec (le moins logique) delete _tab; ?

[therwald]

Et si tu t'en assurais toi-même en faisant le simple test avec (le moins logique) delete _tab; ?

certainement pas!

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
 
float * singleNumber=new float; 
...
delete singleNumber;

, mais

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
 
float* tab=new float[6];
...
delete[] tab;

Ça ne se décide pas en lisant les entrailles de mouton, il y a une logique simple et précise...
@pingouin84k
Excuse moi, j'aurais dû analyser aussi le code du constructeur de copie: tu reproduis le problème du constructeur par défaut!!! Il faut créer un NOUVEAU TABLEAU, de la même taille, et copier le contenu.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
 
private:
  const int TABSIZE=100;
public:
C01::C01(const C01& c01) :
_tab(new float[C01::TABSIZE])
{
  for(int i=0; i<C01::TABSIZE;i++){
    this->_tab[i]=c01._tab[i];
  }
}
}

[pingouin84k]

Et si tu t'en assurais toi-même en faisant le simple test avec (le moins logique) delete _tab; ?

C'est déjà fait!

[pingouin84k]

@pingouin84k
Excuse moi, j'aurais dû analyser aussi le code du constructeur de copie: tu reproduis le problème du constructeur par défaut!!! Il faut créer un NOUEAVU TABLEAU, de la même taille, et copier le contenu.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
 
private:
  const int TABSIZE=100;
public:
C01::C01(const C01& c01) :
_tab(new float[C01::TABSIZE])
{
  for(int i=0; i<C01::TABSIZE;i++){
    this->_tab[i]=c01._tab[i];
  }
}
}

Tu es un chef! Ça marche

[PilloBuenaGente]

Par ailleurs, je n'utilise pas vector (je n'ai pas essayé array) car en faisant un benchmark je me suis aperçu que les opérations sur un membre déclaré comme suit float* m_tab = new float[n] étaient plus rapides (et de loin) que sur son homologue vector... Visiblement le temps d'accès aux éléments est plus court, mais comme je débute, je ne veux pas trop avancer de certitude.

As tu utilisé un reserve() sur ce vector ? Sinon à chaque(pas vraiment) redimensionnement, il doit réalouer son contenu.

[pingouin84k]

As tu utilisé un reserve() sur ce vector ? Sinon à chaque(pas vraiment) redimensionnement, il doit réalouer son contenu.

Je ne connais pas cette fonction.
Est-elle sensée rendre les opérations plus rapides?


Ce que je voulais dire par mon post précédent est que ça:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
for (i = 0; i < n; i++)
{C[i] = A[i] + B[i];}

est plus rapide quand A, B et C sont déclarés comme cela float* X = new float[n]; que comme cela vector<float> X(n);

[Bousk]

plus rapide quand A, B et C sont déclarés comme cela float* X = new float[n]; que comme cela vector<float> X(n);

Je serais curieux de savoir sur quoi tu te bases pour affirmer ça.
avec les bons flags de compilation etc, operator[] sur vector sera plus que certainement inliné et la différence sera alors nulle.

[therwald]

Est-elle sensée rendre les opérations plus rapides?

Ce qu'elle fait, dans la mesure où tu connais d'avance la taille de ton tableau, c'est allouer le stockage en iune seule opération, au lieu d'allouer et réallouer , en recopiant les données chaque fois, au fur et à mesure que le tableau croît. D'où un gain de temps qui peut être assez important.