OVH Cloud OVH Cloud

Est-ce que je m'y prends mal ?

11 réponses
Avatar
Zouplaz
Bonjour, encore besoin d'un coup de main : je vais de crash en crash sans
arriver à déterminer ce qui ne va pas. Une chose est sure, l'outil que
j'utilise pour vérifier les erreurs d'allocation/deallocation me signale
une erreur dans une de mes méthodes sans que je comprenne ce que je peux
bien faire de travers.

Voici une version réduite du code, je voudrais que vous me disiez si ce
que je fais est valide ou pas (je parle du principe, je poste donc une
version simplifiée du code d'origine).

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

J'ai 3 classes :

class Stuff2D
{
public:
Stuff2D(void);
virtual ~Stuff2D(void);
}

class Sprite2D : public Stuff2D
{
public:

Sprite2D(void);
virtual ~Sprite2D(void);
}

class Sprite2DAnimated : public Stuff2D
{
public:
Sprite2DAnimated(void);
virtual ~Sprite2DAnimated(void);
}

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

Et j'ai aussi une classe qui sert de "loader" :

static class StuffLoader
{
public:
StuffLoader(void);
~StuffLoader(void);

Stuff2D* loadStuff(std::string fileName);

private:
typedef std::map<std::string,Stuff2D*> stuffMap;
typedef stuffMap::iterator stuffIterator;

stuffMap m_objects;

} stuffLoader;

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

Maintenant la méthode qui pose problème :

Stuff2D* StuffLoader::loadStuff(std::string fileName)
{
stuffIterator it;

it=m_objects.find(fileName);
if(it==m_objects.end()) // stuff n'existe pas
{
if(fileExists(fileName + ".ani"))
{
Sprite2DAnimated* new_stuff = new Sprite2DAnimated();
new_stuff->load(fileName);
m_objects.insert(std::make_pair(fileName,new_stuff));
return new_stuff;
}
else if(fileExists(fileName + ".bmp"))
{
Sprite2D* new_stuff = new Sprite2D();
new_stuff->load(fileName + ".bmp");
m_objects.insert(std::make_pair(fileName,new_stuff));
return new_stuff;
}
else
return NULL;
}
else
// stuff existe
return (*it).second; // Retourne un pointeur vers l'instance;
}

Qui est appellée de la manière suivante (dans main.cpp par exemple)

Stuff2D* sp1;
sp1 = stuffLoader.loadStuff("spr1");

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

Une erreur est détectée sur la ligne Sprite2D* new_stuff = new Sprite2D
();

L'erreur est "allocation overrun", ce que je traduis par débordement
d'allocation, je suppose que le heap de l'application est endommagé à ce
moment précis (c'est une simple supposition).

Toujours est-il qu'à la sortie de l'application, quand je dois détruire
les instances allouées dynamiquement le heap est réellement endommagé.

Du coup, je me demande s'il est bien possible d'avoir :
- dans main un objet déclaré comme Stuff2D*
et
- dans loadStuff retourner en réalité soit un Sprite2D*, soit un
Sprite2DAnimated*

Y-a-t-il quelque chose d'erroné dans ma manière de procéder ??

Merci

10 réponses

1 2
Avatar
Cyrille Karmann
Zouplaz disait...

Une erreur est détectée sur la ligne Sprite2D* new_stuff = new Sprite2D
();

L'erreur est "allocation overrun", ce que je traduis par débordement
d'allocation, je suppose que le heap de l'application est endommagé à ce
moment précis (c'est une simple supposition).

Toujours est-il qu'à la sortie de l'application, quand je dois détruire
les instances allouées dynamiquement le heap est réellement endommagé.


A part quelques erreurs de syntaxe sûrement apparues quand tu as recopié
le code ici, je ne vois qu'un seul truc qui me gêne: la variable
stuffLoader est une variable static, et c'est généralement une mauvaise
idée. (voir
http://www.ifrance.com/jlecomte/c++/c++-faq-lite/ctors-fr.html#[10.11]
et suivantes.)
Essaye de voir ce qui se passe si stuffLoader est une variable locale.

Sinon, il y a peut-être un problème dans les constructeurs de Stuff2D ou
Sprite2D.

Du coup, je me demande s'il est bien possible d'avoir :
- dans main un objet déclaré comme Stuff2D*
et
- dans loadStuff retourner en réalité soit un Sprite2D*, soit un
Sprite2DAnimated*


A priori faire ça ne devrait poser aucun problème. Le mécanisme de
l'héritage est là pour que tu puisses renvoyer un pointeur sur la classe
de base sans que le code appelant se préoccupe de savoir si c'est en
réalité un Sprite2D ou autre chose.

--
Cyrille

Avatar
Christophe Lephay
Zouplaz wrote:
Sprite2D* new_stuff = new Sprite2D();
new_stuff->load(fileName + ".bmp");
...

Une erreur est détectée sur la ligne Sprite2D* new_stuff = new
Sprite2D ();

L'erreur est "allocation overrun", ce que je traduis par débordement
d'allocation, je suppose que le heap de l'application est endommagé à
ce moment précis (c'est une simple supposition).


Ce serait bien :
1- de montrer le code du constructeur incriminé
2- de le réécrire, le cas échéant, de manière à ce qu'il soit "exception
safe"
3- qu'il renvoie une exception en cas d'echec

et, point subsidiaire :
4- tester la fonction load, au cas où le fichier chargé serait invalide ou
endommagé (indiquer un problème via une valeur de retour ou une exception)

Chris

Avatar
kanze
Zouplaz wrote in message
news:...

Bonjour, encore besoin d'un coup de main : je vais de crash en crash
sans arriver à déterminer ce qui ne va pas. Une chose est sure,
l'outil que j'utilise pour vérifier les erreurs
d'allocation/deallocation me signale une erreur dans une de mes
méthodes sans que je comprenne ce que je peux bien faire de travers.

Voici une version réduite du code, je voudrais que vous me disiez si
ce que je fais est valide ou pas (je parle du principe, je poste donc
une version simplifiée du code d'origine).


[...]
Qui est appellée de la manière suivante (dans main.cpp par exemple)

Stuff2D* sp1;
sp1 = stuffLoader.loadStuff("spr1");

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

Une erreur est détectée sur la ligne Sprite2D* new_stuff = new
Sprite2D();

L'erreur est "allocation overrun", ce que je traduis par débordement
d'allocation, je suppose que le heap de l'application est endommagé à
ce moment précis (c'est une simple supposition).


Ça dépend de l'outil : les meilleurs, comme Purify, signale l'erreur
tout de suite, mais dans bien de cas, tu n'aurais l'indication de
l'erreur à un new ou un delete qui vient après que l'erreur a eu lieu.
Parfois même nettement après.

Je ne sais pas ce dont tu te sers comme outil, mais je chercherais
plutôt dans le code qui précède l'erreur, pour voir si je n'ai pas
débordé d'un bloc d'allocation, ou alloué trop peu.

--
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
Zouplaz
- :

Ça dépend de l'outil : les meilleurs, comme Purify, signale l'erreur
tout de suite, mais dans bien de cas, tu n'aurais l'indication de
l'erreur à un new ou un delete qui vient après que l'erreur a eu lieu.
Parfois même nettement après.

Je ne sais pas ce dont tu te sers comme outil, mais je chercherais
plutôt dans le code qui précède l'erreur, pour voir si je n'ai pas
débordé d'un bloc d'allocation, ou alloué trop peu.



Je me sers de boundschecker et une chose est sure c'est que non seulement
il me trouve cette erreur mais en plus mon application crashes violement
ou se comporte de manière erratique...

J'ai réussi à isoler le problème sans pouvoir l'expliquer.

Comme je l'ai dit j'ai plusieurs classes, dont certaines
(Sprite2DAnimated) sont propriétaire (il faudrait que je trouve un terme
plus approprié) d'instances de SpriteFrameSequence (vecteur de
SpriteFrameSequence*), et les SpriteFrameSequence sont propriétaires de
SpriteFrame (vecteur de SpriteFrame*)...

Bon, j'ai donc dans mon main.cpp réalisé plusieurs expérience pour
déterminer si l'implémentation de ces classes comportait des erreurs.
J'ai donc déclaré un instance de Sprite2DAnimated, qui se charge, qui se
joue et qui se nettoie sans problème. Boundschecker ne trouve plus UN
SEUL memory leak, l'application est parfaitement stable.

Par contre, dès que j'utilise ma "factory" c'est à dire Stuff2DLoader
tout part en vrille et comme cette classe ne comporte qu'une simple
méthode je me dis que le problème doit être par là.
Suivant les conseils donnés par un contributeur je ne l'ai plus déclarée
statique mais par une instance globale.

Le code incriminé est ci dessous (ne considerez pas la logique ou
l'intérêt du truc ou même ses failles (il y en a plusieurs), je cherche
simplement à pour l'instant à comprendre comment ça peut tout crasher).

Franchement, si vous avez une idée ça m'aiderait bien ça commence à
m'énerver de pas trouver la connerie que je fais !!!

Merci

-----------------------------------------------
StuffLoader.h
-----------------------------------------------

class StuffLoader
{
public:
StuffLoader(void);
~StuffLoader(void);

Stuff2D* loadStuff(std::string fileName);

private:
typedef std::map<std::string,Stuff2D*> stuffMap;
typedef stuffMap::iterator stuffIterator;

stuffMap m_objects;

};

extern StuffLoader stuffLoader;

-----------------------------------------------
StuffLoader.cpp
-----------------------------------------------

Stuff2D* StuffLoader::loadStuff(std::string fileName)
{
stuffIterator it;

it=m_objects.find(fileName);
if(it==m_objects.end()) // stuff n'existe pas
{
if(fileExists(fileName + ".ani"))
{
Sprite2DAnimated* new_stuff = new Sprite2DAnimated();
new_stuff->load(fileName);
m_objects.insert(std::make_pair(fileName,new_stuff));
return new_stuff;
}
else if(fileExists(fileName + ".bmp"))
{
Sprite2D* new_stuff = new Sprite2D();
new_stuff->load(fileName + ".bmp");
m_objects.insert(std::make_pair(fileName,new_stuff));
return new_stuff;
}
else
return NULL;
}
else
// stuff existe
return (*it).second; // Retourne un pointeur vers l'instance;
// Ligne ci dessus stupide, c'est la surface qu'il me faut,
// pas un pointeur vers l'instance
// sans dommage mais simplement inutile.
}

Dans le code ci dessus, boundcheck trouve aucun d'overrun sur

Sprite2D* new_stuff = new Sprite2D();

que de sprite chargés (10 sprites = 10 overrun)

-----------------------------------------------
main.cpp
-----------------------------------------------

Stuff2D *sp1;
sp1 = stuffLoader.loadStuff("spr1");

-----------------------------------------------
Sprite2DAnimated.cpp
-----------------------------------------------

void Sprite2DAnimated::loadFrameSequence(std::string
baseFileName,std::string name,int delay)
{
int img_num = 0;
SpriteFrame* spf = NULL;

SpriteFrameSequence* spfs = new SpriteFrameSequence(name);
m_frameSequences.insert(std::make_pair(name,spfs));

bool done = false;
while(!done)
{
/* ----------------------------------- */
/* Ici même string + string + string provoquait des crashs */
/* j'ai été obligé d'utilisé un ostringstream */
/* ----------------------------------- */
std::ostringstream ostr;
ostr << baseFileName << '_' << name << "_000" << img_num;

/* ----------------------------------- */
/* Si comme ci dessous : crash / crash */
/* ----------------------------------- */
spf = (SpriteFrame*)stuffLoader.loadStuff(ostr.str());

/* ----------------------------------- */
/* Si comme si dessous : aucun problème */
/* ----------------------------------- */

//spf = new SpriteFrame();
//bool res = spf->load(ostr.str() + ".bmp");

if(spf) // if (res)
{
spf->setDelay(delay);
spf->setNextFrame(img_num+1);
spfs->addFrame(spf);
img_num++;
}
else
{
done = true;
//delete spf;
}
}
}

Avatar
Rémy
"Zouplaz" a écrit dans le message de
news:
- :

Ça dépend de l'outil : les meilleurs, comme Purify, signale l'erreur
tout de suite, mais dans bien de cas, tu n'aurais l'indication de
l'erreur à un new ou un delete qui vient après que l'erreur a eu lieu.
Parfois même nettement après.

Je ne sais pas ce dont tu te sers comme outil, mais je chercherais
plutôt dans le code qui précède l'erreur, pour voir si je n'ai pas
débordé d'un bloc d'allocation, ou alloué trop peu.



Je me sers de boundschecker et une chose est sure c'est que non seulement
il me trouve cette erreur mais en plus mon application crashes violement
ou se comporte de manière erratique...

J'ai réussi à isoler le problème sans pouvoir l'expliquer.

Comme je l'ai dit j'ai plusieurs classes, dont certaines
(Sprite2DAnimated) sont propriétaire (il faudrait que je trouve un terme
plus approprié) d'instances de SpriteFrameSequence (vecteur de
SpriteFrameSequence*), et les SpriteFrameSequence sont propriétaires de
SpriteFrame (vecteur de SpriteFrame*)...

Bon, j'ai donc dans mon main.cpp réalisé plusieurs expérience pour
déterminer si l'implémentation de ces classes comportait des erreurs.
J'ai donc déclaré un instance de Sprite2DAnimated, qui se charge, qui se
joue et qui se nettoie sans problème. Boundschecker ne trouve plus UN
SEUL memory leak, l'application est parfaitement stable.

Par contre, dès que j'utilise ma "factory" c'est à dire Stuff2DLoader
tout part en vrille et comme cette classe ne comporte qu'une simple
méthode je me dis que le problème doit être par là.
Suivant les conseils donnés par un contributeur je ne l'ai plus déclarée
statique mais par une instance globale.

Le code incriminé est ci dessous (ne considerez pas la logique ou
l'intérêt du truc ou même ses failles (il y en a plusieurs), je cherche
simplement à pour l'instant à comprendre comment ça peut tout crasher).

Franchement, si vous avez une idée ça m'aiderait bien ça commence à
m'énerver de pas trouver la connerie que je fais !!!

Merci

...


Stuff2D* StuffLoader::loadStuff(std::string fileName)

...


/* ----------------------------------- */
/* Si comme ci dessous : crash / crash */
/* ----------------------------------- */
spf = (SpriteFrame*)stuffLoader.loadStuff(ostr.str());



loadStuff rend un stuff2D*, est-ce possible de le caster en SpriteFrame* ???

Rémy


Avatar
Christophe Lephay
Zouplaz wrote:
Par contre, dès que j'utilise ma "factory" c'est à dire Stuff2DLoader
tout part en vrille et comme cette classe ne comporte qu'une simple
méthode je me dis que le problème doit être par là.
Suivant les conseils donnés par un contributeur je ne l'ai plus
déclarée statique mais par une instance globale.


Par curiosité, tu devrais essayer de compiler dans un autre environnement
pour vérifier que c'est pas ton environnement actuel qui est cassé...

Chris

Avatar
Alexandre
<snip>
Avec le code du constructeur de Sprite2D (ainsi que son ancètre) ça serait +
simple...
Avatar
Zouplaz
Alexandre - :

<snip>
Avec le code du constructeur de Sprite2D (ainsi que son ancètre) ça
serait + simple...




Il n'y a rien ou presque dans le constructeur de sprite2D, de mémoire :

m_posX = 0;
m_poxY = 0;
m_surface = NULL;

et celui de Stuff2D est un constructeur vide...

Avatar
Loïc Joly
Zouplaz wrote:
-----------------------------------------------
StuffLoader.h
-----------------------------------------------

class StuffLoader
{
public:
StuffLoader(void);
~StuffLoader(void);

Stuff2D* loadStuff(std::string fileName);

private:
typedef std::map<std::string,Stuff2D*> stuffMap;
typedef stuffMap::iterator stuffIterator;

stuffMap m_objects;

};


Je ne vois pas de destructeur à cette classe, alors que le destructeur
par défaut ne fait pas ce qu'il faut. Je ne lui voit pas non plus de
constructeur de copie, ou d'opérateur d'affectation (ni de mécanisme
empêchant la génération des versions par défaut qui sont ici
probablement inadaptées.



extern StuffLoader stuffLoader;

-----------------------------------------------
StuffLoader.cpp
-----------------------------------------------

Stuff2D* StuffLoader::loadStuff(std::string fileName)
{
stuffIterator it;

it=m_objects.find(fileName);
Petit détail, mais je préfère écrire ces deux lignes en une seule. Ca

évite d'avoir même pour un très court instant un it non initialisé.
En perfs, ça peut aussi potentiellement avoir un impact, mais c'est
souvent plus accessoire.

[...]

/* ----------------------------------- */
/* Ici même string + string + string provoquait des crashs */
/* j'ai été obligé d'utilisé un ostringstream */
/* ----------------------------------- */
std::ostringstream ostr;
ostr << baseFileName << '_' << name << "_000" << img_num;

/* ----------------------------------- */
/* Si comme ci dessous : crash / crash */
/* ----------------------------------- */
spf = (SpriteFrame*)stuffLoader.loadStuff(ostr.str());

/* ----------------------------------- */
/* Si comme si dessous : aucun problème */
/* ----------------------------------- */

//spf = new SpriteFrame();
//bool res = spf->load(ostr.str() + ".bmp");



Ce genre de problème est typique de problème plus haut dans le code qui
bousillent la mémoire mais ne se voient que plus tard.

--
Loïc

Avatar
Alexandre

Il n'y a rien ou presque dans le constructeur de sprite2D, de mémoire :

m_posX = 0;
m_poxY = 0;
m_surface = NULL;

et celui de Stuff2D est un constructeur vide...

Alors il y a un vrai problème. Je ne vois pas du tout dans ton code ce qui

pourrait poser problème (mais ça peut venir d'un bout de code non montré...)
Comme on te l'a suggéré, tu as essayé dans un autre environnement (autre
compilo, autre système, etc...) ?

1 2