Discussion :

[Caidriq]

Voila je cherche depuis quelques jours mais je ne trouve pas la solution. Ce code me renvoie un message "Erreur de segmentation (core dumped)" et j'aimerais savoir si quelqu'un connait la solution?

cpersonne.hpp:

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
class CPersonne{
	private:
	string *nom, *prenom, *id, *mail;
	public:
		CPersonne();
		CPersonne(string, string, string, string);
		void set_nom(string);
		void set_prenom(string);
		void set_id(string);
		void set_mail(string);
		string get_nom() const;
		string get_prenom() const;
		string get_id() const;
		string get_mail() const;
		friend bool operator ==(CPersonne, CPersonne);
		friend ostream& operator<<(ostream& out,CPersonne p);
		~CPersonne();
};

cpersonne.cpp:

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
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
CPersonne::CPersonne(){
	this->nom = new string;
	this->prenom = new string;
	this->id = new string;
	this->mail = new string;
}
 
CPersonne::CPersonne(string nom, string prenom, string id, string mail){
	 this->nom = new string(nom);
	 this->prenom = new string(prenom);
	 this->id = new string(id);
	 this->mail = new string(mail);
 }
 
 void CPersonne::set_nom(string nom){
	 this->nom = new string(nom);
 }
 
void CPersonne::set_prenom(string prenom){
	this->prenom = new string(prenom);
}
 
void CPersonne::set_id(string id){
	this->id = new string(id);
}
 
void CPersonne::set_mail(string mail){
	this->mail = new string(mail);
}
 
string CPersonne::get_nom() const{
	return *(this->nom);
}
 
string CPersonne::get_prenom() const{
	return *(this->prenom);
}
 
string CPersonne::get_id() const{
	return *(this->id);
}
 
string CPersonne::get_mail() const{
	return *(this->mail);
}
 
bool operator ==(CPersonne p1, CPersonne p2){
	return (p1.get_id() == p2.get_id());
}
 
ostream& operator<<(ostream& out,CPersonne p){
	return out<<p.get_nom()<<", "<<p.get_prenom()<<", "<<p.get_id()<<", "<<p.get_mail()<<"."<<endl;
}
 
CPersonne::~CPersonne(){
	delete nom;
	delete prenom;
	delete id;
	delete mail;
}

main.cpp:

Code :

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

1
2
3
4
5
6
7
8
int main(int argc, char const* argv[]){
	CPersonne p1 ("Lau", "Cédric", "1", "xxxx");
	CPersonne p2 ("Cou", "Patrick", "1", "xxx");
	if(p1 == p2){
		cout<<"C'est bon."<<endl;
	}
	cout<<p1;
}

Merci d'avance si quelqu'un a le temps et la solution à m'apporter!

[jo_link_noir]

Ne pas utiliser de pointeurs, c'est la meilleure solution.
Ne pas utiliser new est aussi une excellente solution.
Utiliser std::unique_ptr pour les ressources dynamique est généralement une des meilleurs solutions quand l'allocation "manuelle" est requise (c'est à dire pas souvent).

Ensuite, pourquoi l'erreur ? Parce qu'une copie de la classe (cf: operator<<(ostream &, CPersonne)) va copier les pointeurs, détruire l'objet local en fin de fonction (et libérer les pointeurs) puis détruire l'objet d'origine (en fin de fonction main) et re-libérer les pointeurs. Résultat: de libération, un segfault.
Toujours faire un constructeur de copie profonde (ou le supprimer) pour les classes qui utilisent des ressources allouées dynamiquement.
Utiliser std::unique_ptr pour les pointeurs permet de s'affranchir de la libération manuelle (il le fait pour nous et mieux que nous) et faire une copie aurait fait une erreur de compilation.

Note: paramètre en référence constante std::string -> std::string const &.

[dalfab]

Bonjour,

Le plantage doit certainement arriver à la dernière ligne du code.

* La règle des 3 :
Si on défini un destructeur, un constructeur par copie et/ou un opérateur de copie. Il faut aussi définir les trois.
En créant l'opérateur de copie le plantage devrait disparaître.

* Comment passer les paramètres objet de manière opportune ?
Si on avait passé les CPersonne par const CPersonne&, le plantage ne se serait pas produit.

* Bien utiliser les pointeurs et les new.
Ne pas allouer par new des objets que l'on peut créer directement, sinon on se tend des pièges. Si les allocations dynamiques n'avaient pas été utilisées, il n'y aurait pas eu plantage. Et les deux règles précédentes n'auraient pas manquées.*

grilled

[Caidriq]

Dans ma surcharge de l'opérateur << j'ai changé CPersonne par const CPersonne& et j'ai toujours l'erreur. En ce qui concerne l'utilisation des pointeurs je suis d'accord avec vous sauf que c'est un TP noté et je suis obligé de les utiliser comme inscrit dans le code. Qu'est-ce qu'un opérateur(constructeur?) de copie?

[Bousk]

C'est plus viceux et mal que ça, le crash survient sur la ligne == puisque l'opérateur utilise des copies.
https://en.wikipedia.org/wiki/Operat..._C_and_C%2B%2B

[dalfab]

En effet, le remplacement des paramètres CPersonne en CPersonne const& est à faire dans toutes les fonctions (ici déclaration et définition de operateur<< et de operateur==)
Cela devrait fonctionner.


Mais cela reste incomplet pour un avoir un code valide.


Sur la mauvaise utilisation du new, les fonctions set_nom() set_prenom() set_id() set_mail() sont incorrectes

Code :

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

1
2
3
void CPersonne::set_nom(string nom){
	 this->nom = new string(nom);
 }

Vois-tu pourquoi ? Indice : qui a-t-il dans this->nom au début de la fonction ?

Un constructeur de copie, permet de créer un objet à partir d'un objet du même type, il existe toujours implicitement, on doit en définir explicitement un, surtout s'il y a des allocations dynamiques par :

Code :

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

1
2
3
4
5
6
CPersonne::CPersonne( CPersonne const& p ) {
    nom = new string( p.nom );
    prenom = new string( p.prenom );
    id = new string( p.id );
    mail = new string( p.mail );
}

Quand on veut passer un objet en paramètre comme tu l'as fait, c'est lui qui est utilisé, c'est pourquoi le passage d'objets par const& plus optimal évite de créer cet intermédiaire.

Un opérateur de copie, permet de copier un objet dans un objet de même type, il existe toujours implicitement, il faut aussi le définir par :

Code :

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

1
2
3
4
5
6
7
CPersonne& CPersonne::operator=( CPersonne const& p ) {
    *nom = *p.nom;
    *prenom = *p.prenom;
    *id = *p.id;
    *mail = *p.mail;
    return *this;
}

Il est lui utilisé si on fait p1 = p2, et sa définition ci-dessus, permet d'éviter un plantage dans ce cas.

S'il n'y avait pas d'allocation par new, la gestion directe ou l'utilisation des unique_ptr<> n'auraient pas nécessité toutes ces subtilités. J'ai des doutes sur la bonne chronologie pédagogique de certains cours