Discussion :

[sas]

bonjour à tous, j'ai un ch'ti problème avec une classe servant à lire/écrire dans des fichiers. Cette classe est contenu dans une dll et l'ensemble fonctionne bien sauf (évidement) une fonction membre....

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
 
void
CFileIO::ReadWord(WORD *wBuf)
{
    char *szBuf = NULL;
 
    szBuf = new char[sizeof(WORD) + 1]; 
    if ( szBuf )
    {
        memset(szBuf, 0, sizeof(WORD));
        m_iofile.read(szBuf, sizeof(WORD)); 
 
        memcpy(wBuf, szBuf, sizeof(WORD)); 
        *wBuf = INVW(*wBuf);
 
         delete [] szBuf;            
     }
}        
 
void
CFileIO::ReadString(string &strBuf)
{
       WORD SizeBuf;
       int iSize;
       char *szBuf = NULL;
 
        ReadWord(&SizeBuf);
        iSize = SizeBuf;
        szBuf = new char[iSize + 2];
        if ( szBuf )
        {           
                memset(szBuf, 0, iSize + 1);                        
                m_iofile.read(szBuf, iSize);
 
                strBuf = szBuf;
        }
        delete [] szBuf;
}

La fonction ReadWord marche très bien, je l'utilise ailleurs sans problème.

Le hic c'est quand je debug avec gdb j'ai une erreur de segmentation
sur une instruction du genre :

Code :

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

1
2
3
4
5
6
7
8
 
    CFileIO myfile;
    string strString;
 
    '
    '  ici le code d'ouverture du fichier
    '
    myfile.ReadString(strString); // <- gdb me trouve un SIGSEGV

Voyant la simplicitée du code de la fonction ReadString je ne crois pas qu'il y est d'erreur. Alors dois-je chercher ailleurs dans mon code ?

Ayant passer quelques temps la-dessus j'ai quelques questions qui me viennent :
- dans 'szBuf = new char[iSize + 2];' le 'iSize' peut-etre de quel type (int, WORD, DWORD...) ?
- cette instruction 'strBuf = szBuf;' fonctionne par copie ou non ?
- y a-t-il une version C++ des fonctions 'memset' et 'memcpy' ?

merci d'avance de vos réponses et bonne journée à tous

[philippe V]

Salut

iSize est un int par convention (i)variable pour un int, (b)variable pour un bool, (sz)variable poue un string.....

strBuf = szBuf

STD::string::operator = fait effectivement une copy

memset et memcpy ...

memset: Il y a bien un ZeroMemory() mais je suis pas sur s'y il appartien au MFC ou a Windows oubien si il est standard??? de toute facon en C++ on utilise les constructeurs pour initialiser nos object

memcpy: SL nous fourni un std::copy() mais dans quelle interet???

je ne voit pas de probleme avec la methode ReadString
Peut etre ajouter un test

Code :

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

m_iofile!=NULL

;

[sas]

je suis en train de ré-analyser mon code et j'ai trouver des new sans delete honte à moi... en corrigeant ça tout devrait rentrer en ordre...

merci pour tes réponses (j'ai tendances à douter de mes acquis quand des crashs du genre surviennent, ha ces maudits memory-leaks que d'heures perdues à les dénicher...), sinon existe-il des softs du style BoundCheckers mais adapter à Dev-C++ ???

[sas]

bon je viens de trouver d'ou provenais l'erreur, par contre comprends pas pourquoi il y a cette erreur

en fait mon code ReadString et ReadWord est bien OK, c'est dans le traitement du fichier qu'il y a un bug.

voici le code que j'utilise pour accèder au données d'un fichier :

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
 
enum LOC { lcUNK=0, lcHDD, lcCD };
typedef struct CATALOG_FILE
{
    CATALOG_FILE()
    {
        data= new BYTE[DATASIZE + 2];
        memset(data, 0, DATASIZE + 1);
        Path = "?";
        Genre = "?";
        Titre = "?";
        loc = lcUNK;        
    }
    ~CATALOG_FILE()
    {
       delete [] data;
    }
 
    string Path;
    string Genre;
    string Titre;
    BYTE   *data;
    LOC    loc;
};

juste une petite question sur cette structure, avant de vérifier mon code, j'utilisais juste cette forme là :

Code :

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

1
2
3
4
5
6
7
8
9
 
typedef struct CATALOG_FILE
{
    string Path;
    string Genre;
    string Titre;
    BYTE   data [DATASIZE];
    LOC    loc;
};

(ça bugger pareil), mais laquelle est le mieux ?

voici comment j'accède au fichier :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
    DWORD Count;
    long i;
 
    Cat.ReadDword(&Count);
    for (i = 0; i < Count; i++)
    {
        Cat.ReadString(cf.Path);
        Cat.Read(cf.data, DATASIZE);
 
        list.push_back(cf); << c là que ça plante
    }

en fait 'list' est déclaré comme ça : 'vector <CATALOG_FILE> list;'
et après quelques tours, l'éxécution de 'list.push_back(cf)' fait changer subitement 'cf.data', ensuite le 'i' est incrémenté, puis lors de l'éxécution de 'Cat.ReadString(cf.Path);' çà crash, mais pourquoi ?? je croyais que le vector était sécurisé enfin qu'il ne pouvait pas arriver des trucs du genre....

si qqn voit pkoi il y a un écrasement, des explications seraient les bienvenues car là je ne capte pas.... merci

[Luc Hermitte]

memset et memcpy ...

memset: Il y a bien un ZeroMemory() mais je suis pas sur s'y il appartien au MFC ou a Windows oubien si il est standard??? de toute facon en C++ on utilise les constructeurs pour initialiser nos object

Les éléments de la bibliothèques standard sont tous en minuscules seulement, ou en majuscules seulement quand il s'agit de macros.
Avec le C++, il y a std::fill_n. Ou les vecteurs (et autres containers) qui sont initialisés avec le constructeur qui prend une taille => implicite

memcpy: SL nous fourni un std::copy() mais dans quelle interet???

Plusieurs:
- Type-safe, pas besoin de faire des calculs savants avec sizeof
- Pas de surcout pour les types de base, POD (je crois bien).
- Appelle l'opérateur d'affectation avec les objets. Objets qui ne peuvent et ne doivent en aucun cas être copiés avec memcpy. Le résultat serait un comportement indéfini.
- Compatible avec une écriture itérateurs. Bien quand on use et abuse de ceux-ci dans nos codes. Un peu moins adapté quand on est à un bas niveau de communication et que l'on sérialize et désérialize dans des buffers d'octets.

Ensuite, des remarques en vrac.
- new est censé renvoyer une exception sur les compilateurs à jour, pas null. (par défaut VC6 nous oblige à faire des tests). Mais de toutes façons, c'est inutile car ...
- Pourquoi autant de copies dans readWord ? Il suffit de lire directement dans la variable POD.

Perso, j'aurai tendance à écrire un simple :

Code :

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

1
2
3
4
5
6
template <typename T>
void Read(T & out) {
    m_iofile.read(reinterpret_cast<&out), sizeof(out));
    reorder(out); 
    // à écrire comme une spécialisation template qui dépendra de la machine cible, ...
}

- Les vecteurs ne sécurisent pas contre des mauvaises utilisation. La première sécurité est de ne pas perdre de mémoire en cas d'exception ou de retour hatif.

- Le readString me semble un peu compliqué -- des variables dupliquées, des pointeurs, déclarations anticipées des variables.
J'ai plutôt tendance à partir dans cette direction:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
template <> // spécialisation template
void Read(std::string & out) {
    std::string::size_type size; // le seul vrai type taille pour les chaines
    Read(size);
    if (size) {
        std::vector v(size); // init inutile à 0
        m_iofile.read(&v[0], size);
        out.assign(v.begin(), v.end());
        // il y a deux copies difficiles à éviter avec std::string qui n'assure 
        // pas la consécutivité de ses éléments
    } else { 
        std::string().swap(out); // reset à rien de la chaine
    }
}

- la structure CATALOG_FILE, première écriture, (sur le forum) n'est pas copiable à cause du pointeur brut. L'absence de contructeur de recopie et d'opérateur d'affectation spécialisés interdit toute copie, à cause du problème du double delete.
Le plus simple est de n'utiliser que des attributs copiables. La copiabilité de la classe englobante est alors assurée par une sorte de transitivité.

Bien évidemment tu utilises un container (standard) de données de type CATALOG_FILE. Or les containeurs standard exigent la copie-constructibilité (copie via constructeur de recopie, qu'il soit défini explicitement, ou généré automatiquement par le compilo) pour les éléments stockés. Ton type n'est pas copie constructible, donc ka-boum!

La seconde écriture est copiable si LOC l'est. Les vecteurs peuvent s'avérer mieux qu'un tampon à taille fixe. => base plus conviale.

[sas]

merci pour toutes ces infos, je vois que t'as quelques années d'expérience dans les doigts, par contre dans tes explications j'ai décroché à la dernière partie (trop technique... )

une question : 'reorder(out);' çà fait koi ???

pour le ReadString que tu proposes...ben suis sur le cul... je savais pas que l'on pouvait faire : m_iofile.read(&v[0], size); comme si c'était un tableau de char !!! c'est sur que comme çà c plus simple !!!

par contre est-ce normal comme déclaration :
std::vector v(size); <-- y a pas le type du vector!!!
ou un oubli et je devrais lire :
std::vector <T>v(size);


Pourrais-tu me dire dans quels cas il faut utiliser la copie de classe/structure ? vois pas trop bien pkoi dans le vector <CATALOG_FILE> ça plante...

- la structure CATALOG_FILE, première écriture, (sur le forum) n'est pas copiable à cause du pointeur brut. L'absence de contructeur de recopie et d'opérateur d'affectation spécialisés interdit toute copie, à cause du problème du double delete.
Le plus simple est de n'utiliser que des attributs copiables. La copiabilité de la classe englobante est alors assurée par une sorte de transitivité.

Bien évidemment tu utilises un container (standard) de données de type CATALOG_FILE. Or les containeurs standard exigent la copie-constructibilité (copie via constructeur de recopie, qu'il soit défini explicitement, ou généré automatiquement par le compilo) pour les éléments stockés. Ton type n'est pas copie constructible, donc ka-boum!

La seconde écriture est copiable si LOC l'est. Les vecteurs peuvent s'avérer mieux qu'un tampon à taille fixe. => base plus conviale.

en fait c'est ici que j'ai décroché...donc si tu pourrais etre moins technique m'aiderait a comprendre....

[Luc Hermitte]

J'ai en effet oublié de préciser le type des éléments du vecteur. Ici le but est de récupérer un buffer d'octets. Soit, on va stocker des caractères.

Code :

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

std::vector<char> v(size);

reorder() serait juste une fonction template à écrire et spécialiser soi-même. Le but est de corriger les problèmes d'endianisme liés aux sérialisations "binaires" de nombres. C'était juste pour remplacer de façon générique le INVW.


Sinon, ton problème est celui de la double libération (delete[] dans ton cas) de la ressource (mémoire ici) brute (tu utilises un pointeur).
Quand tu utilises push_back, une copie de l'élément, passé en paramètre, est réalisée au moment de le stocker dans le vecteur.

Soit, cela revient à écrire:

Code :

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

1
2
3
4
{ 
    T t0; // que tu remplis...
    T t(t0);
}

Par défaut le compilo génère un constructeur de recopie qui fait appel aux constructeurs de recopie (ou assimilé) des attributs de ta classe T. Dans ton cas, un des attributs est un pointeur. Sa recopie est juste une recopie d'adresse.

Résultat, tu as t0 et t qui ont tous les deux un attribut de type de pointeur. Ces deux pointeurs ayant la même valeur (pointeur == adresse) après recopie.

Arrivé à l'accolade fermante, t0 et t sont détruits. Dans les destructeurs, tu détruis les variables pointées par tes pointeurs. En fait tu essaies de détruire deux fois la même variable => boum.

Je pense que cela doit être traité dans une FAQ ou une autre. C'est un grand classique.

[sas]

là j'ai tout capté, de plus je viens de lire quelques docs sur les constructeurs par copie... j'avais vu ça avec les templates mais pas fait le rapprochement.

j'apprends le c++ d'une façon assez anarchique, je le reconnais, du coup je bloque pendant des heures sur des trucs tout con.... ben je vais me replonger dans mon code pour en mettre un peu partout de ces 'constructeurs par copie', il en a bien besoin.....

[Luc Hermitte]

Ils ne sont pas nécessaires si tu construis tes classes autour d'attributs copiables (ou interdits de recopie) -- en fait, c'est la bonne technique (une règle simple (iste) pour ne pas faire n'importe quoi -> une classe de haut niveau ne devrait pas posséder (gérer en fait) de ressource brute, et en bas niveau ne pas en posséder plus d'une (ressource brute))

Ils ne sont nécessaires qu'autour de ressources brutes (un vecteur, un smart-pointer, un fichier standard (non copiable en fait)... ne sont pas des ressources brutes). Et quand ils sont nécessaires, alors l'opérateur d'affectation l'est aussi.

Il y a aussi la possibilité de les interdire (la copie & l'affectation)

[sas]

la méthode du buffer avec un vector de char m'a beaucoup plus, mais par contre je sèche sur son adaptation avec des données styles int, long, float....

comment faire pour assigner/récupérer ce genre de données, utiliser le décalage de bits pour chaque élément du vector ?? Ou y a-t-il une solution plus simple ??

[Luc Hermitte]

Je ne suis pas sûr de comprendre ce que tu veux dire.
Je me suis servi du vecteur de char comme d'un simple tampon (auto-libéré et -initialisé) pour récupérer une donnée (non nécéssairement formatté) dont la taille n'était pas connue à la compilation.

Les types de base se récupèrent directement sans passer par un tampon intermédiaire. C'est ce que fait la première fonction template que j'avais donnée.

[sas]

Ben en fait dans une autre librairie de gestion de fichiers j'utilise un tampon de type BYTE pour lire/ecrire des données, je place à la suite des unes des autres, des données de type différent (string, int, DWORD... enfin de tout type ) dans le tampon et j'écris ce tampon dans un fichier (réciproquement pour la lecture).

Ton buffer sous forme de vector m'a plu alors je voudrais adapter mon code pour qu'au lieu d'utiliser un tableau BYTE ce soit un vector de BYTE. Le hic c'est que, par exemple, pour un DWORD comment faire pour le 'loger' dans ce tampon. voici une proposition pour un string :

Code :

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

1
2
3
4
5
 
    std::vector <BYTE> StreamBuf;
 
    for (int i = 0; i < strString.size())
        StreamBuf[m_PosCurrent + i] = (BYTE)strString[i];

et pour un DWORD j'aurais fait :

Code :

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

1
2
3
4
5
6
7
 
    std::vector <BYTE> StreamBuf;
 
    StreamBuf[m_PosCurrent] = ((dwValue << 24) >> 24);
    StreamBuf[m_PosCurrent + 1] = ((dwValue << 16) >> 24);
    StreamBuf[m_PosCurrent + 2] = ((dwValue << 8) >> 24);
    StreamBuf[m_PosCurrent + 3] = (dwValue >> 24);

et dans le même style

Mais vois-tu une autre solution plus intéressante ?