Discussion :

[PierreJssp]

Bonjour à tous !

Je suis étudiant en mathématiques et novice en C++. J'ai pris connaissance, sur un site d'auto formation bien connu, de l'existence de la "fuite de mémoire", et voici mon tourment:

Je crée une classe "matrice" (dans le cadre de mes cours, construction de la classe imposée) et je souhaite faire une fonction qui ajoute une ligne à la matrice, à la fin.

Voici les champs privés de la classe matrice:

Code :

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

1
2
3
int size1;
int size2;
vecteur* mrows;

(j'ai créé une classe vecteur aussi, je sais STL fonctionne très bien mais pas le choix... Cette classe marche très bien)

Voici la fonction désirée :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
matrice matrice :: ajout_ligne() {
    vecteur zero(size2);
    vecteur* stock;
    stock = new vecteur [size1];
    for (int i=0; i<size1; i++) stock[i] = mrows[i];
    size1 = size1 + 1;
    mrows = new vecteur [size1];
    for(int i=0; i<size1-1; i++) mrows[i] = stock[i];
    mrows[size1-1] = zero;
    return *this;
}

Est-ce qu'il y a ici une fuite de mémoire ? Je pense que oui parce que je ne supprime pas l'ancienne valeur du pointeur mrows ainsi la case mémoire utilisée est toujours occupée. Est-ce vrai ce que je dis ?
Si oui, rajouter la ligne delete[] mrows; juste avant mrows = new vecteur [size1]; réglerait le problème ?

Pour être franc je ne suis pas rassuré et j'aimerai que cela s'éclaircisse pour moi.

Merci beaucoup d'avance de prêter attention à ma demande !!

[Médinoc]

Je dirais surtout qu'il y a deux fuites de mémoire: une sur stock et une sur mrows.

Normalement, pour être exception-safe, tu devrais utiliser des std::unique_ptr<> de partout, ainsi que la fonction std::swap() pour échanger entre la variable membre et la variable temporaire (en gros, t'inspirer de l'idiome copy-and-swap).
À ne pas utiliser unique_ptr, tu fais du C++ "à l'ancienne", pas du C++ moderne. Ce qui suggère que la formation proposée par ce site est obsolète.

Edit: Ensuite, côté performance, je ne vois pas l'intérêt d'avoir une fonction dédiée pour rajouter une ligne si à chaque appel elle réalloue tout le tableau de lignes; une fonction plus générique redimensionner_matrice() me semblerait plus appropriée.

[PierreJssp]

Merci pour votre réponse !
Donc est-ce que rajouter un delete[] stock et delete[] mrows supprimerait la fuite de mémoire?

[Bousk]

C'est normal de retourner *this et d'en créer une copie ?
Je suppose que mrows est une variable membre (préfixe m), dont pourquoi voudrais-tu la delete ?
Pourtant je vois size1 et size2 ? Donc pas de préfixe m pour les variables membres ?
stock est bien une variable locale cette fois ?
Tu réalloues sur mrows sans le libérer avant ? Donc son allocation précédente est maintenant perdue et une fuite mémoire ?

(j'ai créé une classe vecteur aussi, je sais STL fonctionne très bien mais pas le choix... Cette classe marche très bien)

De toute évidence elle ne marche pas très bien.

Si en plus tu manipules de la mémoire, je suppose que tu as correctement implémenté constructeur de copie, mouvement, assignation et assignation par mouvement ? Ou ces fonctions ont été correctement supprimées ?
Et le destructeur remplit son rôle proprement ?

[PierreJssp]

Voici mes modifications :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
matrice matrice :: ajout_ligne() {
    vecteur zero(size2); // Création d'un vecteur de size2 0.
    vecteur* stock;
    stock = new vecteur [size1];
    for (int i=0; i<size1; i++) stock[i] = mrows[i];
    size1 = size1 + 1;
    delete[] mrows; // Pour moi, ça permet de libérer la valeur du pointeur mrows
    mrows = new vecteur [size1];
    for(int i=0; i<size1-1; i++) mrows[i] = stock[i];
    mrows[size1-1] = zero;
    delete[] stock; //On libère le pointeur stock
    return *this; //Je retourne la matrice modifiée. 
}

Les champs privés de la classe matrice sont :

Code :

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

1
2
3
4
 
int size1; //nombre de lignes
int size2; //nombres de colonnes
vecteur* mrows;

En fait, ma prof m'a appris qu'un pointeur était un "tableau". Ainsi ici, mrows est un tableau de vecteur (où vecteur est la classe que j'ai créé avant la classe matrice évidemment).
Je ne vois pas l'intérêt d'implémenter constructeur de copie (fait), mouvement (qu'est-ce?), assignation et assignation par mouvement (??).
(Les champs privés de la classe vecteur sont size (nombre d'éléments) et double [] p ("tableau de double") )

[Invité]

Bonjour,

Ce serait nettement plus simple si tu utilisais un std::vector. Ton ajout de ligne se résumerait à appeler sa fonction membre push_back().

Sinon, tu effectues des copies inutiles. Ce que tu fais à la fin pour mrows, c'est ce que tu pourrais faire directement avec stock :

Code :

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

1
2
3
4
vecteur *stock = new vecteur [size1 + 1];
for(int i=0; i<size1; i++)
    stock[i] = mrows[i];
stock[size1] = zero;

puis après avoir désalloué mrows, il te suffirait de le faire pointer à la même adresse :

Code :

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

1
2
3
delete [] mrows;
mrows = stock;
++size1;

Mais niveau fuite, je te laisse imaginer si un seul de tes new (ici ou dans les vecteurs) lance une exception…

Je ne vois pas l'intérêt d'implémenter constructeur de copie (fait), mouvement (qu'est-ce?), assignation et assignation par mouvement (??).

L'intérêt est que si tu ne les implémente pas, ce que va implicitement implémenter ton compilateur sera faux. Il ne fera que copier les adresses et cela donnera lieu à des fuites mémoire, des double libérations, ou à des accès à de la mémoire déjà libérée.
Par exemple pour les vecteurs où tu les as sûrement aussi oubliés, lorsque tu fais stock[i] = mrows[i]; (qui utilise l'opérateur d'affectation par copie) : la zone mémoire pointée par stock[i].p sera perdue (fuite) et les deux pointeurs stock[i].p et mrows[i].p vont pointer sur la même zone mémoire. Cette même zone mémoire que tu libéreras avec delete [] mrows;.

À noter que si tu utilises std::vector, tu n'as pas à t'embêter à en coder un seul, ou à te soucier des fuites. Cf. The rule of three/five/zero et Comment gérer proprement des allocations/désallocations de ressources ? Le RAII !