Discussion :

[deubelte]

Yo!

J'ai une question qui peut paraître bête, mais bon...
J'ai le code suivant:

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
IIterator< UneClasse* > *  buildIter = 0;
//Ok, on instancie sur la stack un pointeur null
 
switch (blablabla)
{
 case 1:
buildIter=fonc1(...);
break;
 case 2:
buildIter=fonc2(...);
break;
 case 3:
buildIter=fonc3(...);
break;
default:
....
}
 
delete buildIter ;
buildIter = 0 ;

On voit ici l'utilisation de delete sur buildIter, mais buildIter n'a pas été instancié sur le tas avec new. Je croyais qu'il fallait utiliser delete quand on instancie un truc sur le tas.

[Bousk]

que font fonc1, fonc2, ... ?
probablement un new.

[deubelte]

Exact, ca marche si on fait :

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
std::list<int>& fussnc(std::list<int> *dfsdf)
{
 
std::list<int>* temporaire=new std::list<int>();
 
return (*temporaire);
}
 
int main()
{  
 
	 std::list<int> *fsdf=0;
     fussnc(fsdf);
	 delete fsdf;
 
}

Dans le cas présent, pas de memory leak. En fait, j'ai tendance à penser que new et delete doivent se faire dans le même scope

[gbdivers]

Bonjour

On critique souvent la mauvaise utilisation des pointeurs... et j'ai l'impression que c'est le cas ici.
- pointeur nu, c'est moche (ton code n'est pas du tout exception free)
- pourquoi un pointeur tout court ?
- pourquoi créer l'objet dans un fonction et pas directement (et passer une référence donc)
- avant de faire le new, il faut vérifier que le pointer n'est pas nul (fuite mémoire potentielle)
- respect de la "règle" : "celui qui créé, c'est celui qui détruit" ?
- il n'y aura pas de différence de coût entre copier un pointeur sur un itérateur ou copier un itérateur (ou faire un swap d'un conteneur)
- pourquoi la fonction fussnc() retourne la liste qui est passé aussi en entrée ?

Bref, c'est à mon sens du code typique à bannir :

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
UnContainer<UneClasse*> c;
IIterator<UneClasse*> buildIter = c.end();
//Ok, on instancie sur la stack un pointeur null
 
switch (blablabla)
{
    case 1:
        buildIter=fonc1(...);
        break;
    case 2:
        buildIter=fonc2(...);
        break;
    case 3:
        buildIter=fonc3(...);
        break;
    default:
        ....
}
// on a fini 
IIterator< UneClasse*>  buildIter = c.end();

(c'est mieux avec l'indentation)

Code :

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

1
2
3
4
5
6
7
8
9
10
void fussnc(std::list<int>& dfsdf)
{
    // on utilise la liste
}
 
int main()
{  
    std::list<int> fsdf;
    fussnc(fsdf);
}

[Bousk]

Dans le cas présent, pas de memory leak.

euh... comment peux-tu affirmer ça ?!

je t'affirmerai même le contraire

Code :

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

1
2
3
4
5
6
7
8
9
10
11
std::list<int>& fussnc(std::list<int> *dfsdf)
{
std::list<int>* temporaire=new std::list<int>();
return (*temporaire);
}
int main()
{
     std::list<int> *fsdf=0;
     fussnc(fsdf);
     delete fsdf; 
}

> le pointeur en paramètre n'est pas utilisé
> le pointeur créé avec new et retourné n'est pas assigné
> tu delete un objet NULL

[Xtrem_Voyageur]

On voit ici l'utilisation de delete sur buildIter, mais buildIter n'a pas été instancié sur le tas avec new. Je croyais qu'il fallait utiliser delete quand on instancie un truc sur le tas.

A priori, cette phrase indique tu es dans le flou.

Reprenons avec un exemple simple où je vais initialiser tout de suite mon pointeur à null pour être dans la même situation que toi.

Ici je crée un objet de type pointeur pi sur un entier sur la pile et l'initialise à null.
pi est donc destiné à recevoir l'addresse d'un entier qui pourra être aussi bien être crée sur la pile ou sur le tas.

Code :

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

1
2
 
int *pi = 0;

Je crée un entier a sur la pile et je fais pointeur pi dessus. Ici, aussi bien mon entier a que mon pointeur d'entier
pi sont créés sur la pile, il n'y a donc aucun delete à faire. Les 2 objets seront détruits automatiquement.

Code :

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

1
2
3
 
int a = 11; 
pi = &a

Maintenant je crée un entier sur le tas, ce qui se fait avec l'opérateur new, on parle d'allocation mémoire dynamique.
Et je fais pointer pi sur cette mémoire

Code :

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

1
2
 
pi = new int(99);

Ici il te faudra faire un delete pi pour libérer la mémoire allouée
pour placer l'entier 99 et non pas détruire le pointeur pi qui a été crée sur la pile et l'est toujours.

Code :

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

1
2
 
delete pi;

En faisant cela, je libère donc la mémoire réservée pour contenir l'entier 99 et c'est tout!

Désolé si je me méprends, mais j'ai l'impression en te lisant que tu penses q'un delete p supprime p, or cela libère la mémoire pointée par p. Le pointeur lui même continue à exister après et sera supprimé automatiquement s'il a été crée sur la pile ou à l'aide d'un delete s'il a lui même été crée sur le tas. En espérant ne pas t'avoir embrouiller.

[deubelte]

Merci pour vos réponses. Je vais essayer de répondre à certaines d'abord.

- pointeur nu, c'est moche (ton code n'est pas du tout exception free)

c'est quoi un pointeur nu?

- pourquoi un pointeur tout court ?

que veux tu dire par pointeur tout court?

- pourquoi créer l'objet dans un fonction et pas directement (et passer une référence donc)

Parce que c'est ce qui se passe dans le code de la fonction avec les switch.

- respect de la "règle" : "celui qui créé, c'est celui qui détruit" ?

Je suis ok. Mais dans le cas de mon premier poste, ce n'était pas le cas. Le code du premier poste ne vient pas de moi (celui avec les switch).

Bref, c'est à mon sens du code typique à bannir :

Ouais, ben c'est du code qui existe dans un logiciel d'une grande boite.


---------------------------------

le pointeur en paramètre n'est pas utilisé

et alors? bon, c'est vrai, c'est pas top.

le pointeur créé avec new et retourné n'est pas assigné

exact. C'est là qu'est la fuite de mémoire. Je suis allé un peu trop vite.

tu delete un objet NULL

Ceci est autorisé par la norme du C++.


Je pense que ceci est mieux:

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
 
std::list<int>* fussnc(void)
{
 
std::list<int>* temporaire=new std::list<int>();
 
return (temporaire);
}
 
int main()
{  
     Fil2 * zff=new Fil2(1,5);
     union DATATYPE Values = { zff };   // iValue = 10
	 std::list<int> *fsdf=0;
     fsdf = fussnc();
	 delete fsdf;
 
}

Désolé si je me méprends, mais j'ai l'impression en te lisant que tu penses q'un delete p supprime p, or cela libère la mémoire pointée par p. Le pointeur lui même continue à exister après et sera supprimé automatiquement s'il a été crée sur la pile ou à l'aide d'un delete s'il a lui même été crée sur le tas. En espérant ne pas t'avoir embrouiller.

Je suis d'accord, mais je vois en quoi consiste la libération de la mémoire pointée par p. Par contre, je vois pas trop ce que signifie "supprimer une variable". Si tu libère la mémoire pointée par un pointeur, alors le pointeur pointe dans le vide. C'est pour cette raison qu'on fait builditer=0 pour pas avoir ce phénomène.