Constructeur par copie...

Bonjour Fanny,

Fanny Chevalier wrote:

et mon constructeur par copie :
Matrix::Matrix(const Matrix &m)
{
for (unsigned int i = 0 ; i < _lines ; i++)
delete [] _matrix[i];
delete [] _matrix;


On m'a appris a nettoyer avant de recoiper mais ca seg fault...
(meme en mettant if (_matrix[i] != NULL)).
Est-ce que c'est necessaire de faire le nettoyage ou pas?
Si oui, qu'est-ce qui va pas?

C'est dans l'operator= qu'il faut libérer la mémoire avant de la réallouer.
Dans le cas du constructeur, la mémoire n'a pas encore été allouée (l'objet
n'existait pas avant l'appel du constructeur), donc pas la peine de la libérer.

Bon courage.

Fanny Chevalier wrote:

Voici les champs prives de ma classe Matrice...

unsigned int _lines;
unsigned int _columns;
double **_matrix;

mon destructeur :

Matrix::~Matrix()
{
for (unsigned int i = 0 ; i < _lines ; i++)
delete [] _matrix[i];
delete [] _matrix;
}

et mon constructeur par copie :
Matrix::Matrix(const Matrix &m)
{
for (unsigned int i = 0 ; i < _lines ; i++)
delete [] _matrix[i];
delete [] _matrix;

Non, il n'y a rien à libérer ici ! Un constructeur est
appelé pour créer un nouvel objet dans une zone de mémoire
où il n'en avait pas avant, donc le pointeur '_matrix'
n'a encore jamais été initialisé - en particulier il ne
pointe pas sur un tableau alloué avec 'new[]', et il
n'y a même pas la garantie qu'il vaille NULL, d'où
les "seg fault".

_lines = m._lines;
_columns = m._columns;

_matrix = new double* [_lines];

for (unsigned int i = 0 ; i < _lines ; i++)
_matrix[i] = new double [_columns];

for (unsigned int i = 0 ; i < _lines ; i++)
for (unsigned int j = 0; j < _columns ; j++)
_matrix[i][j] = (m._matrix)[i][j];
}

On m'a appris a nettoyer avant de recoiper mais ca seg fault...
(meme en mettant if (_matrix[i] != NULL)).
Est-ce que c'est necessaire de faire le nettoyage ou pas?

Tu confonds peut-être avec l'opérateur d'affectation, genre
Matrix& Matrix::operator=(const Matrix &m)
qui, lui, ne peut être appelé que pour affecter une nouvelle
valeur à un objet déjà existant.

Sinon, tu pourrais te simplifier la vie en utilisant std::vector,
voir std::valarray - ces classes gèrent la mémoire tout seul ...

Falk

Fanny Chevalier <chevalie@labri.fr> writes:

[...]

On m'a appris a nettoyer avant de recoiper mais ca seg fault...

Où ?

(meme en mettant if (_matrix[i] != NULL)).

Ce n'est effectivement pas nécessaire. « delete [] 0 » est bien
défini et ne fait rien. Mais si tu n'es pas certaine que la matrice a
bien été allouée avant de la désallouer, tu as un problème. La boucle
for déréférence le pointeur _matrix, en faisant _matrix[ i ]. Il faut
alors tester s'il faut ou non désallouer *avant* le parcours de
_matrix.

En passant, les noms commençant par un '_' sont pour beaucoup
interdits. Les règles sont plus précises que cela, mais une bonne
pratique est de ne jamais utiliser de tels identificateurs. Sauf bien
sûr si tu implémente la bibliothèque standard. Cfr. les archives du
groupe.

Une autre bonne pratique, bien que je pense que tout le monde ne
s'accorde pas dessus, est de mettre un pointeur désalloué à 0. Cela
sert en quelque sorte de marqueur pour dire « je ne suis pas alloué ».
J'ai déjà vu des choses comme :

template < typename T >
void myDelete( T * & p ) {
delete p ;
p = 0 ;
}

Est-ce que c'est necessaire de faire le nettoyage ou pas?

Ca dépend. Veux-tu éviter les fuites mémoire ?-)

Si oui, qu'est-ce qui va pas?

A priori, je ne vois que le problème de déréférencement de _matrix
lors de la désallocation, alors qu'elle n'a pas été allouée. Ceci
devrait résoudre le problème :

Matrix::~Matrix() {
if ( _matrix ) {
for ( int i = 0 ; i < _lines ; ++ i ) {
delete [] _matrix[ i ] ;
}
delete [] _matrix ;
_matrix = 0 ;
}
}

Comme cette opération est commune au destructeur et au constructeur
de copie, j'en ferais une fonction membre privée :

bool Matrix::cleanup() {
if ( _matrix ) {
for ( int i = 0 ; i < _lines ; ++ i ) {
delete [] _matrix[ i ] ;
}
delete [] _matrix ;
_matrix = 0 ;
return true ;
}
else {
return false ;
}
}

Matrix::~Matrix() {
cleanup() ;
}

Matrix::Matrix( Matrix const & rhs ) {
cleanup() ;
// ...
}

Enfin, pour ce genre de choses, je trouve plus simple, et sans doute
plus efficace, d'utiliser une seule allocation, et de calculer le
décalage d'après les indices. Quelque chose comme :

Matrix::Matrix( Matrix const & rhs )
{
int num_of_cells = rhs._lines * rhs._columns ;

if ( ! _matrix || _lines * _columns != num_of_cells ) {
double * new_matrix = new double[ num_of_cells ] ;
delete [] _matrix ;
_matrix = new_matrix ;
_lines = rhs._lines ;
_columns = rhs._columns ;
}

for ( int i = 0 ; i < num_of_cells ; ++ i ) {
_matrix[ i ] = rhs._matrix[ i ] ;
}
}

Matrix::~Matrix()
{
delete [] _matrix ;
_matrix = 0 ;
}

double &
Matrix::at( int l , int c )
{
assert( l >= 0 && l < _lines && c >= 0 && c < _columns ) ;
return _matrix[ l * _columns + c ] ;
}

L'utilisation de `new_matrix' permet d'éviter de modifier l'objet en
cas d'exception lancée par new. Bien que je ne vois pas si cela peut
être utile ...

--drkm, en recherche d'un stage : http://www.fgeorges.org/ipl/stage.html

Hum. Évidemment, comme l'a fait remarquer Falk, le constructeur de
copie est un constructeur, donc rien n'a encore pu être alloué. Il
faut vraiment que je me réveille :-(

Matrix::Matrix( Matrix const & rhs )
: _lines( rhs._lines )
, _columns( rhs._columns )
, _matrix( new double[ _lines * _columns ] )
{
for ( int i = 0 ; i < num_of_cells ; ++ i ) {
_matrix[ i ] = rhs._matrix[ i ] ;
}
}

Désolé pour cette erreur monu-mentale.

--drkm, en recherche d'un stage : http://www.fgeorges.org/ipl/stage.html

drkm <usenet.fclcxx@fgeorges.org> writes:

for ( int i = 0 ; i < num_of_cells ; ++ i ) {

for ( int i = 0 ; i < _lines * _columns ; ++ i ) {

--drkm, en recherche d'un stage : http://www.fgeorges.org/ipl/stage.html

drkm wrote:

Matrix::~Matrix() {
if ( _matrix ) {
for ( int i = 0 ; i < _lines ; ++ i ) {
delete [] _matrix[ i ] ;
}
delete [] _matrix ;
_matrix = 0 ;

Dans un destructeur, c'est pas forcément très utile, non plus. Ça peut
permettre de rendre plus visible l'utilisation d'un objet déjà détruit,
mais il vaudrait mieux s'assurer qu'on utilise pas d'objet déjà détruit.

--
Arnaud
(Supprimez les geneurs pour me répondre)

Fanny Chevalier <chevalie@labri.fr> wrote in message news:<cdo762$ur4$1@news.u-bordeaux.fr>...

Bonjour, j'ai quelques petits problemes quant a la bonne facon de liberer
l'espace memoire occupee par mon objet Matrice :

Voici les champs prives de ma classe Matrice...

unsigned int _lines;
unsigned int _columns;
double **_matrix;


mon destructeur :

Matrix::~Matrix()
{
for (unsigned int i = 0 ; i < _lines ; i++)
delete [] _matrix[i];
delete [] _matrix;
}


et mon constructeur par copie :
Matrix::Matrix(const Matrix &m)
{
for (unsigned int i = 0 ; i < _lines ; i++)
delete [] _matrix[i];
delete [] _matrix;


_lines = m._lines;
_columns = m._columns;

_matrix = new double* [_lines];

for (unsigned int i = 0 ; i < _lines ; i++)
_matrix[i] = new double [_columns];

for (unsigned int i = 0 ; i < _lines ; i++)
for (unsigned int j = 0; j < _columns ; j++)
_matrix[i][j] = (m._matrix)[i][j];
}


On m'a appris a nettoyer avant de recoiper mais ca seg fault...
(meme en mettant if (_matrix[i] != NULL)).
Est-ce que c'est necessaire de faire le nettoyage ou pas?
Si oui, qu'est-ce qui va pas?

Le nettoyage est nécessaire dans un opérateur d'affectation.
Mais que veux-tu nettoyer dans un objet en construction ?

Matrix::Matrix(const Matrix &m)
{
for (unsigned int i = 0 ; i < _lines ; i++)
delete [] _matrix[i];
delete [] _matrix;

Ici tu fais des delete sur des pointeurs non-initialisés (et _lines a
une valeur indeterminée). Ce code est inutile.

Bertrand.

Arnaud Meurgues <arnaud@meurgues.non.fr.invalid> writes:

drkm wrote:

Matrix::~Matrix() {
if ( _matrix ) {
for ( int i = 0 ; i < _lines ; ++ i ) {
delete [] _matrix[ i ] ;
}
delete [] _matrix ;
_matrix = 0 ;

Dans un destructeur, c'est pas forcément très utile, non plus. Ça peut
permettre de rendre plus visible l'utilisation d'un objet déjà
détruit, mais il vaudrait mieux s'assurer qu'on utilise pas d'objet
déjà détruit.

La première fois que j'ai vu une telle chose, il me semble que
c'était dans un article de Sutter (mais là, je ne suis pas sûr). Cela
m'avait étonné, au premier abord. Mais je trouve finalement que pour
un coût minime, cela permet parfois de se rendre compte, au moyen
d'une sortie de debug ou d'un debuger, que l'on accède à un pointeur
non alloué. Je pense que c'est intéressant justement pour provoquer
une faute de segmentation si l'on essaie de déréférencer ce pointeur.
J'imagine que :

int * p = new int ;
delete p ;
* p = 0 ;

passerait sans problème à l'exécution dans beaucoup de cas (outre
l'avertissement à la compilation de tout compilateur digne de ce nom,
j'imagine). Mais pas :

int * p = new int ;
delete p ;
p = 0 ;
* p = 0 ;

--drkm, en recherche d'un stage : http://www.fgeorges.org/ipl/stage.html

drkm wrote:

Désolé pour cette erreur monu-mentale.

--drkm, en recherche d'un stage : http://www.fgeorges.org/ipl/stage.html

Et avec ce genre d'erreurs, tu espères en trouver ? O;p

--
Loïc

On Thu, 22 Jul 2004 15:06:20 +0200, drkm <usenet.fclcxx@fgeorges.org>:

--drkm, en recherche d'un stage : http://www.fgeorges.org/ipl/stage.html

Le projet sur lequel je travaillerai dans le
cadre de ce stage doit
comporter de l'analyse et de la programmation.

Analyse de la machine à café et programmation d'icelle pour que le
café soit prêt quand tout le monde arrive le matin, ça te convient ?


--
;-)