Discussion :

[Kalemiro]

Bonjour, je veux lire une image PGM (P3 ou P5) et stocker les composantes de gris de chaque pixels dans une matrice (qui est assimilée à un tableau à 2 dimensions).
J'ai une classe matrice qui crée un tableau dynamique de la taille de l'image (largeur et hauteur en pixel), voici ma classe (enfin juste le début):

Code :

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

1
2
3
4
5
6
 
class Matrice{
private:
   unsigned int taille_x; //Width
   unsigned int taille_y; //Height
   unsigned int **tableau; //Matrix

Je crée donc une instance de la classe avec la largeur et la hauteur de l'image et le tableau est vide, tout va bien. Ensuite j'utilise ma fonction pour lire le PGM:

Cette fonction prend le nom du fichier en paramètre, elle lit d'abord le magic number (p5 ou p3), puis les dimensions de l'image, ensuile la valeur max pour un pixel (255 niveau de gris) et enfin je lis le reste du fichier et je remplis le tableau avec les valeurs de gris:

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
 
void Matrice::lirePGM(std::string filename){
   std::ifstream file(filename.c_str(), std::ios::binary);
   unsigned int w, h, g;
   char extension[2];
   file >> extension;
   if(extension[0] == 'P' && (extension[1]=='5' || extension[1]=='3'))
   {
       skip_line(file);
       skip_comments(file);
       file>>w>>h;
       skip_line(file);
       skip_comments(file);
       file>>g;
       if(g!=255) std::cout << "Error grayscale" << std::endl;
       skip_line(file);
       skip_comments(file);
   }
   else{
       w = h = 0;
       std::cerr << "Error PGM magic number" << std::endl;
   }
   file.read((char*)this->tableau, this->getY()*this->getX()); //Partie pour lire le reste du fichier et le stocker
   file.close();
}

Les dimensions, le magic number ainsi que la valeur max de gris marche bien mais il semblerait que la partie pour lire les pixels ne marche pas. Quand je veux afficher mon tableau il est vide et j'ai une erreur "Erreur de segmentation (core dumped)".

Je précise que j'ai testé ma fonction d'affichage sur une autre matrice (avec des valeurs aléatoires dans la matrice), elle marche bien.

Voici un screen du début de mon fichier PGM:
https://i.gyazo.com/5d4291597cd1d024...28813c9db9.png

Du coup je vois pas ce qui ne marche pas, surement dans ma lecture

[foetus]

recherche le new unsigned int[w][h] (*) et tu auras ta réponse


Édit : Je n'ai pas vérifié , trop de C, et apparemment cette écriture n'est pas permise. Celle ci est correcte new unsigned int[w * h]

[Kalemiro]

J'ai pas compris ce que tu veux dire

[bacelar]

Digression : les pointeurs nus c'est caca, alors, comment dire, "unsigned int **" ça sent pas la rose.

@foetus te demandes, indirectement, de nous prouvez que toutes les cases mémoires dont le contenu qui va être écraser par "file.read((char*)this->tableau, this->getY()*this->getX())" ont bien été réservées pour cet usage (de barbare).

"new unsigned int[w][h]" est une manière (toute moisie) de réserver ces cases mémoires.

[Kalemiro]

@bacelar donc si par exemple je te dit que j'ai réussi à le remplir avec des nombres aléatoires sans erreur (et donc 100 nombres pour une matrice 10*10) ca suffit à prouver que toutes les cases sont allouées ?

Tu connais une méthode moins moisie de les réserver ?

[Bousk]

Il me semble surtout que new unsigned int [w][h] est très faux et ne crash pas uniquement parce que par hasard h < w
Et pour une image à fortiori on fera une allocation à plat de w*h et pas un tableau multidimensionnel

[bacelar]

@bacelar donc si par exemple je te dit que j'ai réussi à le remplir avec des nombres aléatoires sans erreur (et donc 100 nombres pour une matrice 10*10) ca suffit à prouver que toutes les cases sont allouées ?

NON, cela ne prouve rien.
T'as juste modifié la valeur de 10*10*sizeof(char) cases mémoires dans une zone où les flags de protection des pages mémoires autorisent l'écriture, ce qui est le cas de n'importe quel page mémoire mappée, que cela soit du code, la stack, le tas de la RuntimeC, etc...
Donc la probabilité de réussite, même avec une valeur dans "tableau" complètement aléatoire ne sont pas négligeable, très loin de là.
Cela ne prouve que tu maitrises vraiment très mal la notion de gestion mémoire en C++, et même en C (vue ton affection maladive pour les pointeurs nus).

Tu connais une méthode moins moisie de les réserver ?

WTF ?
Bin, nos remarques, c'est sur le fait que tu ne montres en rien la manière donc tu réserves la mémoire.
Et comme on connait bien les peaux de banane des pointeurs nus, on fait l'assertion que t'as suivis des cours de C (le ++ dans le titre, c'est pour appâter le chaland) et que t'as cru qu'en C++, c'est comme dans le C des années 1970.

Tu t'es pris un "Erreur de segmentation (core dumped)" parce que tu gères la mémoire comme une nouille et tu nous montres pas comment tu la gères, cette mémoire.

Donc comment tu réserves la mémoire que tu écrabouilles avec fread ?

T'inquiètes pas, des manières robustes et simple de gérer la mémoire en C++, c'est pas cela qui manque, mais c'est fonction de comment tu comptes te servir de la mémoire après.

[Kalemiro]

Ok donc plutôt que de prendre de haut il serait peut être temps de redescendre un peu sur terre, et de montrer un peu plus de compréhension plutot que balancer des accusations à tout va . Je n'ai JAMAIS fait de C, et j'ai appris le C++. Ne prend rien de personnelle dans ces phrases, elles sont valables pour pas mal de gens sur les forums..

Tu veux peut être voir comment je crée le tableau ? Si c'est ca je te le met pas de soucis:

Code :

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

1
2
3
4
5
6
7
8
 
Matrice::Matrice(unsigned int& _taille_x, unsigned int& _taille_y)
       :taille_x(_taille_x), taille_y(_taille_y)
{
  tableau = new unsigned int* [_taille_y]; //Initialisation du tableau
  for(unsigned int i = 0; i < _taille_y; ++i)
    tableau[i] = new unsigned int [_taille_x];
}

Maintenant si j'affiche le tableau directement après son instanciation, il est rempli de 0.

Si jamais tu te pose la question de la "désalocation" de ce tableau, la voila:

Code :

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

1
2
3
4
5
6
7
8
 
Matrice::~Matrice(){
  //Libere espace mémoire tableau
  for(unsigned int i=0; i < taille_y; ++i){
    delete[] tableau[i];
  }
  delete[] tableau;
}

J'espère que ca a répondu à ta question ?

[Bousk]

@bacelar donc si par exemple je te dit que j'ai réussi à le remplir avec des nombres aléatoires sans erreur (et donc 100 nombres pour une matrice 10*10) ca suffit à prouver que toutes les cases sont allouées ?

int toto[100]; et voilà j'ai un tableau de 100 nombres aléatoires sans erreur

Matrice(unsigned int& _taille_x, unsigned int& _taille_y) c'est quoi ces références ?!

[Kalemiro]

Bousk, c'est le constructeur de matrice on passe la taille du tableau que l'on veut en paramètre, c'est si choquant que ca ?

[foetus]

Code :

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

1
2
3
4
5
6
7
8
 
Matrice::Matrice(unsigned int& _taille_x, unsigned int& _taille_y)
       :taille_x(_taille_x), taille_y(_taille_y)
{
  tableau = new unsigned int* [_taille_y]; //Initialisation du tableau
  for(unsigned int i = 0; i < _taille_y; ++i)
    tableau[i] = new unsigned int [_taille_x];
}

Pour allouer un tableau de w*h entiers (soit w*h*sizeof(int)), tu alloues w*(h*sizeof(int) + sizeof(int*)).

Teins j'ai un doute sur son utilisation parce que tableau est un tableau de pointeurs tableau[10][2] ? (tableau[10])[2] ? *((tableau[10]) + 2) ? ...

[bacelar]

Matrice(unsigned int& _taille_x, unsigned int& _taille_y) c'est quoi ces références ?!

On n'est pas encore à discuter des bonnes pratiques, chaque chose en son temps.

C'est le constructeur de matrice on passe la taille du tableau que l'on veut en paramètre, c'est si choquant que ca ?

Un peu oui. Soit on passe une copie (par défaut, mais ça coute un bras pour les gros objets), soit une référence (non const) pour que le code de la méthode puisse faire des modifications sur l'objet en paramètre, soit en référence const pour que le code de la méthode ne puisse pas faire de modification (cela coûte moins chère qu'une copie pour les gros objets, mais plus pour les petits donc à n'utiliser que sur les gros).
Indice : "unsigned int" est un très petit "objet".

Je n'ai JAMAIS fait de C, et j'ai appris le C++.

Alors, on te la très très mal apprit, désolé. Vraisemblablement un professeur de C à qui on n'a confié un cours de C++. Bien trop fréquent, malheureusement.
On n'utilise jamais des pointeurs nus quand on peut faire sans. (et en beaucoup plus simple, en plus).

Tu veux peut être voir comment je crée le tableau ? Si c'est ca je te le met pas de soucis:

La vache, non offense, mais c'est encore pire que ce que l'on pouvait imaginer.

Celui qui t'as fait apprendre ça, c'est un putain de saligot, et pas nous.

Reprends la lecture d'un bon cours moderne de C++. Un cours où les pointeurs intelligents et les exceptions sont abordés largement avant les pointeurs nus.

Veux-tu vraiment une explication par le menu de toutes les boulettes de ce code ?

Franchement, laisses tomber, il n'y a vraiment rien à y récupérer de cette classe "Matrice". Tu trouveras sur le net des implémentations bien plus simple et correcte, à base de std::vector<>.

Si tu veux vraiment une explication de pourquoi cela marche de temps en temps, va falloir t'accrocher au pinceau parce que je vais retirer l'échelle.

"tableau" pointe sur une zone mémoire gérée par la C-Runtime. Elle y a réservée "_taille_y" * sizeof(unsigned int*) plus vraisemblablement quelques octets pour la gestion de la C-Runtime. "sizeof(unsigned int*)", c'est fonction de l'architecture cible (32/64bits ...). On va nommer cette zone mémoire contiguë la "racine".
Chacun des cellules mémoires de la "racine" contient un pointeur vers une autre zone mémoire gérée par la C-Runtime, qui ne pas forcement contiguës, voir très hautement improbable.
Donc en mémoire, vous avez une case mémoire (le champ tableau) qui pointe sur une zone mémoire (la zone racine) avec à l'intérieur de cette zone racine des pointeurs qui pointent vers "_taille_y" autres zones mémoire de _taille_x * sizeof(unsigned int) de long (plus quelques octets pour la gestion de la C-Runtime).
Votre zone allouée tient plus de la toile d'araignée que du gros pavé en mémoire.

Est-ce qu'on n'est d'accord ?

Parce que, "file.read((char*)this->tableau, this->getY()*this->getX())", lui, il marche comme un train, bien bourrin, il copie tout à la suite, comme un gros bloque.
Tu lui indique comme adresse de "racine", bin lui, il écrit dans "racine" et dégage tous les pointeurs qui sont dedans.

L'appel de delete sur cas "qui est tombé en marche" (la page de mémoire de "racine" était assez grande pour que le "train" de file.read ne touche pas une zone protégée), ça doit être assez horrible à voir.

[Kalemiro]

La création apparemment désastreuse de mon tableau à deux dimensions vient tout simplement d'ici:
https://fr.wikibooks.org/wiki/Progra...aux_dynamiques

Et si mon code ne vaut rien, alors comment feriez-vous pour créer une classe matrice permettant de faire des opérations (calcul de la covalence...) SANS passer par une bibliothèque ? Une simple idée générale suffira ..

Et je ne comprends toujours pas le problème avec mon constructeur

[JolyLoic]

Je n'ai pas lu tous les messages, mais la manière idiomatique de gérer un tableau de taille variable en C++, c'est std::vector.

Certes, c'est une bibliothèque, mais la bibliothèque standard livrée avec chaque compilateur. Donc, si c'était pour une histoire de simplicité que tu voulais éviter une bibliothèques, ça ne devrait pas te retenir d'utiliser std::vector.

Si c'est pour une histoire d'apprentissage, je te conseille quand même très fortement d'utiliser std::vector plutôt que de l'allocation manuelle de mémoire. Pour les raisons suivantes :
- C'est plus simple
- Le compilateur va pouvoir mieux t'aider sur tout un tas d'erreurs que tu es susceptible de faire
- C'est de toute façon ce qu'un développeur C++ utilise tout le temps, alors que les tableaux et allocations manuelle de mémoire ne sont utiles que dans de très rares cas (quand on implémente std::vector, par exemple). Je ne crois pas m'en être servi au cours des 10 dernières années sauf pour me connecter à du code (souvent legacy) écrit par d'autre personnes. Donc autant apprendre ce qui est vraiment utile.

Ensuite, une fois cet outil bien maîtrisé, il sera temps de refaire le même exercice avec une allocation manuelle de la mémoire. Parce qu'à ce moment, ça sera instructif de voir ce que la gestion manuelle implique (et de remercier du fond du coeur std::vector de nous éviter d'avoir à nous en soucier). Mais juste à titre d'exercice, pour voir. Le vrai code continuera à utiliser un std::vector !

Dernier détail, pour un tableau à deux dimensions à n lignes et m colonnes, on peut faire un std::vector<std::vector<T>>. Mais il est généralement bien plus simple (et plus performant aussi), d'utiliser un simple std::vector<T>, ayant comme taille n*m.

[Matt_Houston]

Je pensais qu'on était cash sur le forum C, mais vous êtes pire ici en fait.

Mais il est généralement bien plus simple (et plus performant aussi), d'utiliser un simple std::vector<T>, ayant comme taille n*m.

Ouaip surtout qu'il s'agit d'une image, on a donc intérêt à la stocker « à plat » (comme le dit Bousk plus haut) pour se faciliter la sérialisation / désérialisation. C'est classique chez les débutants de vouloir utiliser du multidimensionnel pour le traitement d'image, je me demande si ce n'est pas ce qu'on leur enseigne..


@Kalemiro, pour faire simple : évite les tableaux / vector multidimensionnels. C'est une facilité d'écriture mais ça complique plein d'autres choses, d'autant plus si tu ne maîtrises pas ce qui se passe derrière. De toute façon la mémoire est adressée de façon linéaire dans un ordinateur (en tout cas pour une machine de Von Neumann). Alloue un bloc de largeur * hauteur et adresse-le par exemple comme suit :

Code :

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

1
2
Pixel getPixel(const Image& img, std::size_t x, std::size_t y) // ou std::ssize_t pour faire du "wrapping" avec un modulo
{ return img.data[y * img.width + x]; }

[foetus]

Je pensais qu'on était cash sur le forum C, mais vous êtes pire ici en fait.

Et dis-nous (comme JolyLoic) comment tu fais pour créer un bloc mémoire contiguë de w*h en C++ sans new

Ou alors, il faudra refaire la procédure de lecture et écriture Par exemple, lire caractère par caractère et faire un push_back (avec le resize reserve qui va bien avant)
Si on fait un resize, il faut directement mettre la valeur dans l'image img[x][y] = color.

[Matt_Houston]

Ben, resize.. mais je ne comprend pas la question, ou plutôt je ne vois pas le rapport.

Pour du push_back (mauvaise idée), ce n'est pas plutôt reserve qu'il faudrait invoquer ?

[foetus]

Ben, resize.. mais je ne comprend pas la question, ou plutôt je ne vois pas le rapport.

Tu viens vomir sur ce fil (mais tu n'es pas le seul) comme quoi c'est du méga ultra caca C

Mais tu n'as donné aucun code/ classe qui permette de créer une matrice/ tableau2D/ image et lire et écrire dans un fichier en C++

Pour du push_back (mauvaise idée), ce n'est pas plutôt reserve qu'il faudrait invoquer ?

Au temps pour moi

[Matt_Houston]

T'es en roue libre, mec. Je crois que t'as besoin d'un autre kawa, et surtout bois-le bien frais parce que je vois toujours pas de quoi tu causes.

[bacelar]

C'est trolldi aujourd'hui.

Je n'ai rien contre le C, mais on n'apprend pas le C++ comme le C, point barre.

La création apparemment désastreuse de mon tableau à deux dimensions vient tout simplement d'ici:
https://fr.wikibooks.org/wiki/Progra...aux_dynamiques

Il n'y a rien de tout ce bidule d'allocation en chaine dans le document que vous citez, et en plus, quelques ligne plus bas les std::vector sont mentionnés.
Je pense que vous êtes assez prompt à rejeter l'erreur sur cet article.

Moi, je pense que vous n'avez même pas les bases, et c'est pas une critique, mais faut commencer par marcher avant de voler.
Utiliser un simple std::vector mono-dimensionnel comme base. Ou mieux, utilisez des classes matrices toutes faites.

alors comment feriez-vous pour créer une classe matrice permettant de faire des opérations (calcul de la covalence...)

Si vous avez besoin de fonctionnalités en plus, pas de problème, vous les encapsulés avec de l'héritage (privée si vous utilisez les conteneurs de la STL, ils ne sont pas conçus pour l'héritage public) ou de l'encapsulation.

SANS passer par une bibliothèque ?

Et pourquoi pas sans compilateur, tant qu'on n'y est.
La STL fait partie du langage, depuis C++03 ou C++11, je sais plus.
Vous pouvez faire des copier-coller du code pour les versions non template, et juste l'include d'un fichier d'en-tête pour les versions template, les dépendances avec d'autres éléments de la bibliothèque sont probablement nulles ou faibles.

Et je ne comprends toujours pas le problème avec mon constructeur

WTF !!!
Vous avez lu, et compris, mon laïus sur le tas et tout le toutime ?
C'est pas juste pour décorer.
Qu'est-ce qui n'est pas clair ? Je ne suis pas un pédagogue né.