OVH Cloud OVH Cloud

[BCB6 /STL] Violation d'acces dans un classe et sa collection

39 réponses
Avatar
Olivier
Bonjoiur
je fait une collection d'article (std::vector<TArticle *> _pCoArt;)

le probleme quand je supprime un article
ou quand il est le dernier et que je veuille en ajouter un
autre j'ai une violation d'acces

voici le code de la suppression :


bool __fastcall TCoArticles::DeleteArticle(TArticle& Art)
{//@CODE_347

bool bRes=false;

if(int max =Count()>0){

int i=0;

if(SearchArticle(Art,i)){

TArticle *_pArt=_pCoArt.at(i);

if(i==max || max==1){
_pCoArt.push_back();
}else{
_pCoArt.erase(&_pArt);
}
delete _pArt;
bRes=true;
}
}else{
TMyError Err(7,"Collection d'article est vide");
throw Err;
}
return bRes;
}//@CODE_347


le code SearchArticle

bool __fastcall TCoArticles::SearchArticle(TArticle& Art, int& i)
{//@CODE_357
bool bRes=false;
std::vector<TArticle*>::iterator i_end =_pCoArt.end();
int j=0;

for(std::vector<TArticle*>::iterator it=_pCoArt.begin();it!=i_end;++it){

if((*it)->NomArt==Art.NomArt && (*it)->Description==Art.Description &&
(*it)->Categorie==Art.Categorie && (*it)->Prix==Art.Prix){

i=j;
bRes=true;
break;

}
j++;
}

return bRes;
}//@CODE_357

ou est mon erreur ?
que dois je faire ?

merci de vos reponses....
--
Cordialement ,
Sarda Olivier

Site Web:
http://membres.lycos.fr/osarda

9 réponses

1 2 3 4
Avatar
kanze
"Christophe Lephay" wrote in message
news:<bggf5p$g24$...
"Olivier" a écrit dans le message de
news:3f2b9187$0$10532$
Tout d'abord merci de vos precieux conseil
je suis un autodidacte qui souffre avec la STL dur dur



Alors, la première chose à faire, c'est d'acheter un bon bouquin.

_pCoArt est devenu _pArtList

voici le nouveau code dans la classe collection TCoArticle

std::vector<TArticle*>::const_iterator
TCoArticles::SearchArticle(TArticle & Art)
^^^^^^^^^^ ^^^^^^



Tu compte modifier le paramètre chez l'appelant ? Sinon, il vaut mieux
déclarer la référence const. Tu comptes modifier l'objet TCoArticles
dans cette fonction ? Sinon, il vaut mieux déclarer la fonction const.

{
return std::find(_pArtList.begin(),_pArtList.end(),&Art);



Attention ici. La STL travaille sur des valeurs. Dans ce cas-ci, les
valeurs ont un type pointeur. La récherche donc est pour un pointeur
égal à l'adresse du paramètre. Je ne suis pas sûr si c'est ce que tu
veux. (En fait, vue le programme avant, je suis prèsque sûr que ce n'est
pas ce que tu veux.)

Il faudrait donc fournir un prédicat.

}

La suppression :

bool __fastcall TCoArticles::DeleteArticle(TArticle& Art)}
^^^^^^^^^



Ici aussi, j'imagine qu'un const ne serait pas de trop.

{
bool bResúlse;
std::vector<TArticle*>::const_iterator i;
i= SearchArticle(Art);

if(i){


SearchArticle renvoie maintenant un itérateur. Pour tester si
l'élément a été trouvé ou non, utilises plutôt :

if( i != _pArtList.end() )


En effet. En fait, avec une bonne implémentation de la STL, son code ne
se compile même pas. Avec une mauvaise, il se compile, mais l'if serait
toujours vrai.

_pArtList.erase( _pArtList.begin()+ (i -_pArtList.end())) ;


Puisque i est maintenant un itérateur, tu peux faire :

_pArtList.erase( i );


Non. Parce que i est un const_iterator, et pour Dieu sait quelle raison,
erase démande un itérateur non const. Il faut donc :

_pArtList.erase( _pArtList.begin() + (i - _pArtList.begin()) ) ;

(A priori, vue l'expression, on s'attendrait à ce que le résultat soit
le même que i, tout court. Mais si l'élément désigné est bien le même,
le type ne l'est pas.)

Par ailleurs, je te conseille de le renommer en "it" plutôt que "i"
(qui laisse penser que c'est un indice)...


Je conseillerais un coup sérieux de renommage. Tout ces _p, etc. ne me
plaît guère.

delete *i; // violation pourquoi ??? que le mette avant ou apres le errse


Peut-être que tu rentres dans le bloc même quand l'élément a été
trouvé (voir plus haut), ce qui fait que tu tentes d'effacer un
élément via un itérateur invalide.


Ça serait une raison. Aussi, s'il fait le delete après l'érase, il y a
aussi un problème -- parce que *i n'est plus défini. Selon la norme, il
y a aussi un problème s'il le fait avant, mais raisonablement, il n'y a
pas d'implémentation aujourd'hui où ça poserait de problèmes. La
solution 100% correcte serait néaumoins :

T* p = *i ;
_pArtList.erase( _pArtList.begin() + (i - _pArtList.begin()) ) ;
delete p ;

}else{

bResúlse;
}


Si tu mets à false quand l'élément n'a pas été trouvé, il faut mettre
à true quand il l'a été. Par ailleurs, tu peux te passer completement
de cette clause else vu que tu initialises à false au début de ta
fonction...


--
James Kanze GABI Software mailto:
Conseils en informatique orientée objet/ http://www.gabi-soft.fr
Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, +33 (0)1 30 23 45 16


Avatar
kanze
Luc Hermitte wrote in message
news:...

"Olivier" wrote in
news:3f2cddaf$0$27407$:

Projet fait sur Borland C++Builder 6 et Windows 98 code guard active
option de complition standard faut il changer une option ? autre
precison outil UML utilise ClassBuilder 2.4 gratuit generation du
squellette des classes uniquement


Il a fait des progrès cet outil ? Dans le passé, son mainteneur ne
voulait pas supporter la STL et imposait à la place ses propres
conteneurs.


En fait, il ferait bien d'obtenir une bonne bibliothèque récente, avec
un mode de déboggage. Est-ce que la STLPort ne marcherait-elle pas avec
son compilateur ?

Avec une bonne implémentation de la STL, son code ne serait même pas
compilé. (On ne peut pas se servir d'un itérateur comme condition dans
un if, on ne peut pas affecter NULL à un itérateur, etc.)

bool TCoArticles::DeleteArticle(TArticle& Art)


bool TCoArticles::DeleteArticle(const TArticle& Art)

{
bool bResúlse;

if (Count()>0){ // size()
std::vector<TArticle*>::const_iterator it=NULL;
it= SearchArticle(Art);


std::vector<TArticle*>::iterator it > std::find_if(_pArtList.begin(),_pArtList.end(),
std::bind2nd( IsEqualPointedTo<TArticle>(), &Art));

// if(it!=_pArtList.end()){ // ce test ne marche pas il le
saute // par contre si je fait


Si il le saute, ça veut dire qu'il n'a rien trouvé.


En fait, il n'a probablement pas besoin du test de Count() non plus. Vue
qu'il fait la même chose si le tableau est vide que quand il ne trouve
pas l'objet.

D'autant que c'est le seul et unique test valide à faire. "if (it)"
n'a pas de sens. it est un iterateur, pas un pointeur. Déjà
l'initialiser à NULL (ou même 0) n'a pas de sens. [bon d'accord, ici
il est implémenté 99% du temps comme un pointeur ; mais ce servir de
cet aspect d'implémentation est sémantiquement faux]


99% du temps, je ne sais pas. Dans les implémentations récentes, je
crois que le pointeur est en récul.

if(it){ // ce test passe mais sur delete *it j'ai une violation
d'acces
_pArtList.erase( _pArtList.begin()+ (it -_pArtList.end())) ;


_pArtList.erase(it); // tout simplement

delete *it;
bRes=true;
}
}


Le problème est que ton Article n'a pas été trouvé car tu faisais une
recherche sur des pointeurs d'articles et pas des articles. Cf. juste
après pour ce qui te manque.

file://----------------------------------------------------------------
-----
----------------------------------------------------------------------

Autre question ai je les bon header ?
#include <vector>
#include <iterator>
#include <algorithm>
#include <functional>


A priori, tu n'as pas besoin ici de <iterator>; mais bien de
<functional> car il te faut rajouter ce qui suit pour pouvoir comparer
des articles ensembles (et pas des adresses d'articles) :

template< typename T >
struct IsEqualPointedTo
: std::binary_function<const T*, const T*, bool>
// James, tu avais oublié l'héritage avec binary_function ;)


En effet.

{
bool operator()( T const* lhs, T const* rhs ) const {
return *lhs == *rhs ;
}
} ;

bool TArticle::operator==(const TArticle &a) const {
return Categorie_==a.Categorie_ && Description_==a.Description_
&& NomArt_==a.NomArt_;
// Rem: j'ai ignoré le prix ici


J'aurais ignoré la description aussi:-).

}

file://----------------------------------------------------------------
----- ----------------------------------------------------------------
Si je fait _pArtList.erase(it)

bcb=>
[C++ Erreur] CoArticles.cpp(123): E2285 Impossible de trouver une
correspondance pour '_STL::vector<TArticle *,_STL::allocator<TArticle
*>
::erase(TArticle * const *)'


Le selecteur de fonction attend pArtList.erase(TArticle **__position)
ou pArtList.erase(TArticle
**__first,TArticle **__Last)

ou est l'erreur ?


Enlève le const_ de const_iterator


Non. Parce que SearchArticle doit bien être une fonction const, et
renvoyer un itérateur const. Il faut en fait qu'il convertisse
l'itérateur const en itérateur non const -- pour un vector ou un deque,
le code que j'ai montré (v.begin() + (i - v.begin())) fait l'affaire.
Pour un list, je ne sais pas en fait comment le faire.

--
James Kanze GABI Software mailto:
Conseils en informatique orientée objet/ http://www.gabi-soft.fr
Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, +33 (0)1 30 23 45 16



Avatar
Gabriel Dos Reis
writes:

| Gabriel Dos Reis wrote in message
| news:...
| > writes:
|
| [...]
| > | Je ne vois qu'un problème. Problème que je crois n'apparaît pas dans
| > | le code réel, mais que la norme permet. Si on fait dépendre la façon
| > | que le paramètre est passé de ce que le constructeur de copie soit
| > | inline ou non, il y a une risque qu'il soit inline où la fonction
| > | est appelée, et non inline où la fonction est définie.
|
| > C'est une violation des presciptions explicites sur ce que doit être
| > une fonction inline.
|
| > 7.1.2/4
| >
| > An inline function shall be defined in every translation unit in
| > which it is used
|
| Exact. Le point de mon exemple, c'est que les constructeurs de la classe
| ne servent pas dans l'unité de traduction où est définie la fonction
| appelée.

il faut lire le paragraphe en entier,

| > and shall have exactly the same definition in every case
| > (3.2). [Note: a call to the inline function may be encountered
| > before its defi-nition appears in the translation unit. ] If a
| > function with external linkage is declared inline in one
| > transla-tion unit, it shall be declared inline in all translation
| > units in which it appears; no diagnostic is required. An inline
| > function with external linkage shall have the same address in all
| > translation units. [...]
|
| > [...]
|
| > | Je crois que de tel code est légal.
|
| > Non, le code n'est pas valide.
|
| Je crois que si.

Non, il ne l'est pas parce que la norme requiert

If a function with external linkage is declared inline in one
translation unit, *it shall be declared inline in all translation
units in which it appears*; no diagnostic is required.

En particulier dans f.cc, le constructeur de copie est déclaré inline
alors dans dans g.cc, elle ne l'est pas. C'est bien une violation des
dispositions de la norme et le paragraphe que je cite s'applique bien.

[...]

| C'est évidemment un cas qui ne se présente pas dans la pratique. Mais

C'est évidemment un cas bugué qui a ce qu'il mérite.

-- Gaby
Avatar
kanze
Gabriel Dos Reis wrote in message
news:...
writes:

| Gabriel Dos Reis wrote in message
| news:...
| > writes:

| [...]
| > | Je ne vois qu'un problème. Problème que je crois n'apparaît pas
| > | dans le code réel, mais que la norme permet. Si on fait dépendre
| > | la façon que le paramètre est passé de ce que le constructeur de
| > | copie soit inline ou non, il y a une risque qu'il soit inline où
| > | la fonction est appelée, et non inline où la fonction est
| > | définie.

| > C'est une violation des presciptions explicites sur ce que doit
| > être une fonction inline.

| > 7.1.2/4

| > An inline function shall be defined in every translation unit in
| > which it is used

| Exact. Le point de mon exemple, c'est que les constructeurs de la
| classe ne servent pas dans l'unité de traduction où est définie la
| fonction appelée.

il faut lire le paragraphe en entier,


J'ai lu ce que tu as cité. Malheureusement, je ne crois pas avoir saisi
toutes les nuances.

| > and shall have exactly the same definition in every case
| > (3.2). [Note: a call to the inline function may be encountered
| > before its defi-nition appears in the translation unit. ] If a
| > function with external linkage is declared inline in one
| > transla-tion unit, it shall be declared inline in all translation
| > units in which it appears; no diagnostic is required. An inline
| > function with external linkage shall have the same address in all
| > translation units. [...]


D'accord. Il faut qu'elle soit déclarée inline au moins, même si la
définition n'est pas présente.

| > [...]

| > | Je crois que de tel code est légal.

| > Non, le code n'est pas valide.

| Je crois que si.

Non, il ne l'est pas parce que la norme requiert

If a function with external linkage is declared inline in one
translation unit, *it shall be declared inline in all translation
units in which it appears*; no diagnostic is required.

En particulier dans f.cc, le constructeur de copie est déclaré inline
alors dans dans g.cc, elle ne l'est pas. C'est bien une violation des
dispositions de la norme et le paragraphe que je cite s'applique bien.


D'accord. Mais il n'y a toujours pas d'exigence à ce que la définition
soit présente. Alors, si on veut que le passage par régistre se fait en
fonction de la sémantique du constructeur, on n'est pas avancé.

[...]

| C'est évidemment un cas qui ne se présente pas dans la pratique.
| Mais

C'est évidemment un cas bugué qui a ce qu'il mérite.


Le cas que tu décris apparaît en fait assez souvent dans mon code, non
pour les constructeurs dont il est question ici, mais pour d'autres
fonctions. Comme tu peux constater, je viens d'apprendre qu'il faut que
la fonction soit déclarée inline partout -- j'étais convaincu que si on
ne s'en servait pas dans une module, ça faisait rien. Alors, il
m'arrive, même assez souvent, à déclarer des fonctions membre privée, et
à les définir inline dans le .cc -- puisque les fonctions sont privées,
elles ne peuvent servir que dans le .cc, et j'évite ainsi d'en exposer
l'implémentation dans mon .hh. Mais d'après ce que je viens d'apprendre,
il va falloir que je les déclare inline dans le .hh quand même.

--
James Kanze GABI Software mailto:
Conseils en informatique orientée objet/ http://www.gabi-soft.fr
Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, +33 (0)1 30 23 45 16

Avatar
Gabriel Dos Reis
writes:

[...]

| > il faut lire le paragraphe en entier,
|
| J'ai lu ce que tu as cité. Malheureusement, je ne crois pas avoir saisi
| toutes les nuances.

Encore un peu d'effort est on y est.

| > | > and shall have exactly the same definition in every case
| > | > (3.2). [Note: a call to the inline function may be encountered
| > | > before its defi-nition appears in the translation unit. ] If a
| > | > function with external linkage is declared inline in one
| > | > transla-tion unit, it shall be declared inline in all translation
| > | > units in which it appears; no diagnostic is required. An inline
| > | > function with external linkage shall have the same address in all
| > | > translation units. [...]
|
| D'accord. Il faut qu'elle soit déclarée inline au moins, même si la
| définition n'est pas présente.

Oui.

[...]

| > En particulier dans f.cc, le constructeur de copie est déclaré inline
| > alors dans dans g.cc, elle ne l'est pas. C'est bien une violation des
| > dispositions de la norme et le paragraphe que je cite s'applique bien.
|
| D'accord. Mais il n'y a toujours pas d'exigence à ce que la définition
| soit présente. Alors, si on veut que le passage par régistre se fait en
| fonction de la sémantique du constructeur, on n'est pas avancé.

on a avancé. Ce qu'il faut bien voir, c'est que inline en C++
est un changement d'ABI. Beaucoup de gens semblent ignorer ce point.

Ce que inline change ici, c'est de savoir si la fonction g() doit
attendre son argument dans le registre ou dans le tas. Parce que le
constructeur de copie est déclaré inline (même s'il n'est pas
utilisé), il instruit directement le compilateur de faire le choix
d'ABI correspondant.

[...]

| l'implémentation dans mon .hh. Mais d'après ce que je viens d'apprendre,
| il va falloir que je les déclare inline dans le .hh quand même.

Yep.

-- Gaby
Avatar
Alain Naigeon
"Gabriel Dos Reis" a écrit dans le message
news:

ABI


?

--

Français *==> "Musique renaissance" <==* English
midi - facsimiles - ligatures - mensuration
http://anaigeon.free.fr | http://www.medieval.org/emfaq/anaigeon/
Alain Naigeon - - Strasbourg, France

Avatar
Gabriel Dos Reis
"Alain Naigeon" writes:

| "Gabriel Dos Reis" a écrit dans le message
| news:
|
| > ABI
|
| ?

Application Binary Interface

-- gaby
Avatar
Alain Naigeon
"Gabriel Dos Reis" a écrit dans le message
news:
"Alain Naigeon" writes:

| "Gabriel Dos Reis" a écrit dans le
message

| news:
|
| > ABI
|
| ?

Application Binary Interface


Thanks

--

Français *==> "Musique renaissance" <==* English
midi - facsimiles - ligatures - mensuration
http://anaigeon.free.fr | http://www.medieval.org/emfaq/anaigeon/
Alain Naigeon - - Strasbourg, France

Avatar
kanze
Gabriel Dos Reis wrote in message
news:...
writes:

[...]

| > En particulier dans f.cc, le constructeur de copie est déclaré
| > inline alors dans dans g.cc, elle ne l'est pas. C'est bien une
| > violation des dispositions de la norme et le paragraphe que je
| > cite s'applique bien.

| D'accord. Mais il n'y a toujours pas d'exigence à ce que la
| définition soit présente. Alors, si on veut que le passage par
| régistre se fait en fonction de la sémantique du constructeur, on
| n'est pas avancé.

on a avancé. Ce qu'il faut bien voir, c'est que inline en C++ est un
changement d'ABI. Beaucoup de gens semblent ignorer ce point.

Ce que inline change ici, c'est de savoir si la fonction g() doit
attendre son argument dans le registre ou dans le tas. Parce que le
constructeur de copie est déclaré inline (même s'il n'est pas
utilisé), il instruit directement le compilateur de faire le choix
d'ABI correspondant.


Peut-être bien, mais quelle partie de l'ABI ? Est-ce que c'est le fait
que le constructeur de copie de C est inline change l'ABI de :

void f( C copied ) ;

?

Qu'en penses-tu de l'exemple suivant :

C.hh :
#ifndef C_hh
#define C_hh
struct C
{
inline C( C const& other ) ;
inline C( int i = 0 ) ;
~C() ;

int myI ;
} ;
#endif

C.tcc:
#ifndef C_tcc
#define C_tcc
#include "R.hh"

C::C( C const& other )
: myI( other.myI )
{
r.enrol( this ) ;
}

C::C( int i )
: myI( i )
{
}
#endif

R.hh:
#ifndef R_hh
#define R_hh
#include <vector>

class C ;

class R
{
public:
void enrol( C const* p ) ;
void deenrol( C const* p ) ;
bool isPresent( C const* p ) const ;

typedef std::vector< C const* >
Table ;
Table t ;
} ;

extern R r ;
extern void f( C copied ) ;
#endif

C.cc:
#include "C.hh"
#include "C.tcc"
#include "R.hh"
#include <algorithm>

C::~C()
{
r.deenrol( this ) ;
}

R.cc:
#include <algorithm>
#include <cassert>
#include "C.hh"
#include "R.hh"

void
R::enrol( C const* p )
{
t.push_back( p ) ;
}

void
R::deenrol( C const* p )
{
Table::iterator i = std::find( t.begin(), t.end(), p ) ;
if ( i != t.end() ) {
t.erase( i ) ;
}
}

bool
R::isPresent( C const* p ) const
{
return std::find( t.begin(), t.end(), p ) != t.end() ;
}

void
f( C copied )
{
assert( r.isPresent( &copied ) ) ;
}

main.cc:
#include "C.hh"
#include "C.tcc"
#include "R.hh"

R r ;

int
main()
{
C c( 1 ) ;
f( c ) ;
return 0 ;
}

Au moment de la compilation de f, le compilateur sait que les C::C sont
inline, mais il ne sait pas que celui qui va être appeler va faire fuire
son adresse. Alors :

- Est-ce que ce programme est légal ? Je crois que si, mais il se peut
bien qu'il y a quelque chose qui m'a échappé.

- Si oui, est-ce qu'on doit passer « copied » dans des régistres ?
Surtout, selon quel critère est-ce que le compilateur doit faire le
choix ?

- Si on passe « copied » dans des régistres, comment fonction le
constructeur de copie ?

- Si on ne le passe pas, comment est-ce qu'on peut passer quoique ce
soit dans les régistres, étant donné qu'il faut que les deux côtés
s'entende, et que côté appelé, on ne peut pas voir l'implémentation
des constructeurs ?

[...]

| l'implémentation dans mon .hh. Mais d'après ce que je viens d'apprendre,
| il va falloir que je les déclare inline dans le .hh quand même.

Yep.


Encore des corrections dans le code à mon site.

D'un côté, je n'aime pas la réstriction. Je préfèrerais déclarer mes
fonctions privée sans l'inline, et ne fait la décision inline ou non
dans les sources de l'implémentation -- c'est encore une dépendance au
niveau des sources. De l'autre part, si c'est nécessaire pour qu'on
puisse passer des petits objets dans des régistres, ça vaut le prix, et
je m'y plierai.

--
James Kanze GABI Software mailto:
Conseils en informatique orientée objet/ http://www.gabi-soft.fr
Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, +33 (0)1 30 23 45 16

1 2 3 4