Discussion :

[Jipété]

Bonjour,

Le code concerné vient d'un exemple de Dr Dobbs (de 1995, ça nous rajeunit pas) :

Code :

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

1
2
    if (colorTable != NULL)
	free(colorTable);

Au début de la proc :

Code :

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

1
2
    RGB              *colorTable;
    colorTable = (RGB *)calloc(numColors, sizeof(RGB));

Dans un .h :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
/*
 * RGB defines a single color palette entry.  Its analogues in the Windows SDK
 * are the RGBTRIPLE and RGBQUAD structures.  Its analogues in the OS/2
 * Toolkit are the RGB and RGB2 structure. 
 */
typedef struct RGB
{
    UINT8 blue;
    UINT8 green;
    UINT8 red;
} RGB;

Si je remplace la ligne free(colorTable); par printf("Hello, World!\n"); je vois bien le message --> j'en conclus que colorTable est bien différent de NULL, alors pourquoi tant de haine et de violence à l'exécution du code d'origine ?

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
*** glibc detected *** ./testBMP.exe: free(): invalid pointer: 0xb77bdff4 ***
======= Backtrace: =========
/lib/i386-linux-gnu/i686/cmov/libc.so.6(+0x70c91)[0xb76cdc91]
/lib/i386-linux-gnu/i686/cmov/libc.so.6(+0x724f8)[0xb76cf4f8]
/lib/i386-linux-gnu/i686/cmov/libc.so.6(cfree+0x6d)[0xb76d263d]
./testBMP.exe[0x804ab1a]
./testBMP.exe[0x8048a7e]
/lib/i386-linux-gnu/i686/cmov/libc.so.6(__libc_start_main+0xe6)[0xb7673e46]
./testBMP.exe[0x80486a1]
======= Memory map: ========
08048000-0804c000 r-xp 00000000 08:02 926278     /chemin/Dr_Dobbs/prog/testBMP.exe
0804c000-0804d000 rw-p 00003000 08:02 926278     /chemin/Dr_Dobbs/prog/testBMP.exe
09c6d000-09c8e000 rw-p 00000000 00:00 0          [heap]
b7500000-b7521000 rw-p 00000000 00:00 0 
b7521000-b7600000 ---p 00000000 00:00 0 
b7624000-b7640000 r-xp 00000000 08:02 4325509    /lib/i386-linux-gnu/libgcc_s.so.1
b7640000-b7641000 rw-p 0001b000 08:02 4325509    /lib/i386-linux-gnu/libgcc_s.so.1
b765c000-b765d000 rw-p 00000000 00:00 0 
b765d000-b77bc000 r-xp 00000000 08:02 4325472    /lib/i386-linux-gnu/i686/cmov/libc-2.13.so
b77bc000-b77be000 r--p 0015e000 08:02 4325472    /lib/i386-linux-gnu/i686/cmov/libc-2.13.so
b77be000-b77bf000 rw-p 00160000 08:02 4325472    /lib/i386-linux-gnu/i686/cmov/libc-2.13.so
b77bf000-b77c2000 rw-p 00000000 00:00 0 
b77db000-b77df000 rw-p 00000000 00:00 0 
b77df000-b77e0000 r-xp 00000000 00:00 0          [vdso]
b77e0000-b77fc000 r-xp 00000000 08:02 4330281    /lib/i386-linux-gnu/ld-2.13.so
b77fc000-b77fd000 r--p 0001b000 08:02 4330281    /lib/i386-linux-gnu/ld-2.13.so
b77fd000-b77fe000 rw-p 0001c000 08:02 4330281    /lib/i386-linux-gnu/ld-2.13.so
bfa6f000-bfa90000 rw-p 00000000 00:00 0          [stack]
Abandon

J'ai lu et relu le tuto, la seule différence avec mon cas c'est qu'il s'agit d'une structure de 3 UINT8 : ça change beaucoup de choses ?

Je me suis paluché le man malloc :

Code :

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

1
2
3
4
5
6
void *calloc(size_t nmemb, size_t size);
 
  The calloc() function allocates memory for an array of nmemb elements of size bytes each 
  and returns a pointer to the allocated memory. The memory is set to zero. 
  If nmemb or size is 0, then calloc() returns either NULL, or a unique pointer value 
  that can later be successfully passed to free().

Quelqu'un peut m'aider à dépatouiller ça ?
Merci et bon dimanche,

[Matt_Houston]

Mis à part le cast de l'adresse renvoyée par calloc qui n'a pas lieu d'être, il n'y a en effet pas d'incohérence flagrante dans ce que tu nous as présenté. J'en déduis que le défaut se situe au sein de la partie de programme que nous ne voyons pas : peux-tu nous donner plus d'informations ?

Deux remarques :

• utilise le débugger ;
• vérifie dans l'ordre le bon déroulement des opérations : ce n'est pas au moment où tu n'en as plus besoin qu'il faut tester la validité de l'adresse renvoyée par *alloc.

[Jipété]

Merci pour ce premier retour.

J'en déduis que le défaut se situe au sein de la partie de programme que nous ne voyons pas :

C'est ce que je me dis aussi, mais alors, ce test if (colorTable != NULL) ne vaut rien ?
Ceci étant dit, dans les lignes qui précédent, il y a d'autres free (colorTable); suivis directement par return(une_valeur); et je n'y passe pas puisque je me retrouve plus bas.

peux-tu nous donner plus d'informations ?

Ça va être compliqué (plein de code, et je ne veux pas vous prendre la tête avec du code, j'ai surtout besoin de comprendre ma question de la ligne précédente, en fait), d'autant plus qu'il me semblait que ma méthode était suffisante :

Code :

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

1
2
3
4
printf("Hello, World1!\n"); // message vu
    if (colorTable != NULL)
	free(colorTable);
printf("Hello, World2!\n"); // message pas vu et erreur

Code :

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

1
2
3
4
printf("Hello, World1!\n"); // message vu
    if (colorTable != NULL)
//	free(colorTable);
	printf("Hello, World2!\n"); // message vu, suite du prog est fonctionnelle

Deux remarques :

• utilise le débugger ;

Avec le Bloc-notes (de Linux) ça va être tendu...

Et avec gcc je ne sais pas (le C n'est pas mon langage naturel, je cause plutôt Pascal...), d'où mes printf("message ici\n"); c'est bien aussi, des fois j'espionne une variable comme ça

Deux remarques :

• vérifie dans l'ordre le bon déroulement des opérations : ce n'est pas au moment où tu n'en as plus besoin qu'il faut tester la validité de l'adresse renvoyée par *alloc.

Vi, mais ce n'est pas moi qui ai écrit le code ainsi...
Et quand tu dis ... au moment où tu n'en as plus besoin qu'il faut tester... , j'insiste : quid du test if (colorTable != NULL) alors ?


Le commentaire qui précède le test :

Code :

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

1
2
3
4
5
    /*
     * Clean up and return.  Note that we don't return the color table.  This
     * is because we're returning an array of RGB values for the image - such
     * a table would be redundant.
     */

et après on sort de la fonction avec return 0;, fonction qui commence comme ça (pas de quoi fouetter un chat) :

Code :

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

1
2
3
4
5
6
/*
 * readSingleImageBMP will read a single BMP image and generate an array of RGB
 * triples that contain the RGB values for every pixel.  It will also return
 * the dimensions of the image.
 */
int readSingleImageBMP(FILE *fp, RGB **argb, UINT32 *width, UINT32 *height)

[dalfab]

Bonjour,
L'erreur indique que le pointeur colorTable ne correspond pas à quelque chose qui a été alloué par malloc(), calloc() ou realloc().
Alors, ou bien cela vient du fait que

• On n'est jamais passé par un malloc(), calloc() ou realloc(), le pointeur serait aléatoire.
• free(colorTable); a déjà été appelé, et on a oublié de mettre colorTable à NULL.
• ou bien qu'il y a eu écrasement de la variable colorTable (sans debuggeur tu peux mettre un printf("debut %p",colorTable); après l'allocation et un printf("fin %p",colorTable); avant la libération, ils doivent être égaux.)
• peut-être aussi, a-t-on utilisé plus de mémoire que réservé lors du calloc().

[Pyramidev]

Code :

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

1
2
    if (colorTable != NULL)
	free(colorTable);

Dans la norme du langage C de 1989, free(NULL) n'a aucun effet.
Néanmoins, avant que le C soit normalisé, il existait des architectures sur lesquelles free(NULL) provoquait un crash.
Donc le test du pointeur non nul avant d'appeler free avait un sens à une époque, mais est redondant aujourd'hui.

Plus d'infos :
* http://stackoverflow.com/questions/1...corrupt-memory
* https://www.winehq.org/pipermail/win...er/031544.html

[Jipété]

Halte au feu, les copains, j'ai trouvé !

Et dalfab est passé dessus sans voir le code, pas mal !

L'erreur indique que le pointeur colorTable ne correspond pas à quelque chose qui a été alloué par malloc(), calloc() ou realloc().
Alors, ou bien cela vient du fait que

• On n'est jamais passé par un malloc(), calloc() ou realloc(), le pointeur serait aléatoire.

C'est bien ça ! Je bricole avec des bitmaps 24 bits (ce vieux prog ne gère pas les 32) et au début de la fonction il y a un truc genre

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
    /*
     * Allocate and read the color table.  The file pointer has been
     * positioned in the right place by the readBitmapHeader function.  Note
     * that images with 24-bits or more color depth have no color table.  They
     * are already RGB. When reading the color table, be sure to check for
     * old/new format bitmaps. 
     */
printf("%d depth1\n", depth); // 24
    if (depth < 24)
    {
printf("%d depth2\n", depth); // pas vu avec mes fichiers 24 bits donc dessous pas exécuté non plus ! Bingo !
      numColors = 1 << depth;
      colorTable = (RGB *)calloc(numColors, sizeof(RGB));
      ...
    }

La bonne question (qui va à l'encontre de tout ce que je connaissais à ce jour) c'est : comment se fait-il que plus loin le test if (colorTable != NULL) renvoie True ?

Vais étudier la réponse de Pyramidev, dans l'attente je le modifie en if ( (depth < 24) && (colorTable != NULL) ), testé, vendu, résolu !

Merci à tous, bon aprème,

[no. 26]

....
La bonne question (qui va à l'encontre de tout ce que je connaissais à ce jour) c'est : comment se fait-il que plus loin le test if (colorTable != NULL) renvoie True ?
...

Si je ne m'abuse, le pointeur n'a pas été initialisé à NULL lors de sa déclaration, il a dès lors une valeur aléatoire. D'où la bonne pratique de toujours initialiser un pointeur à NULL quand son allocation est décalé et sujet à condition.

Concernant le débogueur, quand tu n'as pas d'EDI sous la main, tu as possibilités sous Linux d'utiliser gdb.

Bonne journée à toi.

[Jipété]

Si je ne m'abuse, le pointeur n'a pas été initialisé à NULL lors de sa déclaration, il a dès lors une valeur aléatoire. D'où la bonne pratique de toujours initialiser un pointeur à NULL quand son allocation est décalée et sujette à condition.

Comme ça ?

Code :

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

1
2
3
4
5
6
7
8
9
int readSingleImageBMP(FILE *fp, RGB **argb, UINT32 *width, UINT32 *height)
{
    BITMAPFILEHEADER  bfh;
    BITMAPHEADER      bh;
//    RGB              *colorTable, *image;
    RGB              *colorTable = NULL;
    RGB              *image = NULL;
    ...
}

Pas mal, je n'y ai pas pensé (je ne pratique vraiment quasiment pas de C, je ne pouvais pas y penser), en tout cas ça fonctionne aussi !
Bien vu

PS : édite ton post et enlève les espaces surnuméraires après CODEINLINE.

[no. 26]

Oui cela semble être cela.
Au vue de ce que tu nous montres comme code depuis tantôt, je suppute que le soucis vient d'une variable non initialisé.

Merci pour l'espace supplémentaire, je me demandais depuis tantôt "pourquoi" les balises s'affichaient.

[Jipété]

Plus d'infos :
* http://stackoverflow.com/questions/1...corrupt-memory

Lecture intéressante pour un néophyte comme moi, mais force est de constater que personne ne parle de l'initialisation à NULL comme no. 26, alors question :
est-ce obligatoire, comme je l'ai mis en œuvre dans mon post #8 après la lecture de dalfab ?
Je n'ai pas souvenir de ce genre d'initialisation en Pascal (Delphi il y a longtemps, Lazarus /FreePascal maintenant).

Et si c'est évident et obligatoire, pourquoi cela n'était pas présent dans le code que j'étudiais, venant du défunt DrDobbs ?

Car mine de rien, il y en a un qui s'est pris 17 points chez S.O. en écrivant

there were some versions of free that would crash on free(NULL) which is why you may see some defensive programming techniques recommend:

Code :

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

1
2
if (ptr != NULL)
    free(ptr);

qui est exactement la construction qui a initié cette discussion, ici et maintenant.

[Sve@r]

mais force est de constater que personne ne parle de l'initialisation à NULL comme no. 26, alors question :
est-ce obligatoire, comme je l'ai mis en œuvre dans mon post #8

Bonjour

On peut étendre la question à "est-il obligatoire d'initialiser une variable à sa création". Moi, ma réponse est toute simple et se base sur deux axiomes

1. je ne remplis jamais deux fois une variable sans l'avoir traitée entre les deux remplissages. De fait, dans le code suivant
   

   Code c :

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

   1
   2
   3
   char *pt=NULL;
   ... (instructions ne faisant aucune référence à "pt")...
   pt=malloc(...);

   L'initialisation à NULL est totalement inutile
   
2. je ne traite jamais une variable sans l'avoir remplie auparavant. De fait, j'ai écrit récemment un code de ce style
   

   Code c :

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

   1
   2
   3
   4
   5
   6
   7
   8
   9
   FILE *fp=NULL;
    
   ...
    
   // Fermeture éventuel fichier précédemment ouvert si existant
   if (fp) fclose(fp);
    
   // Ouverture nouveau fichier
   fp=fopen(...);

   Dans ce code, si ne n'initialise pas "fp" ça ne peut pas fonctionner.

Cela n'est pas à 100% sans risque (surtout quand le code évolue et qu'on rajoute par exemple une référence à "pt" dans le premier cas ou bien quand on développe en équipe) mais ça permet déjà de répondre à pas mal de situtions...

[Pyramidev]

@Jipété :
Quand une variabe n'est pas initialisée, elle a une valeur indéterminée, jusqu'à ce qu'on écrive dedans.

Code :

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

1
2
3
4
int var; // variable non initialisée, donc ayant une valeur indéterminée
int toto = var; // toto a une valeur indéterminée.
var = 5;
int titi = var; // titi vaut 5.

Le danger des comportements indéterminés, c'est d'avoir des bogues qu'on n'arrive pas à reproduire.

A part ça, dans une vieille version du langage C (celle de 1989), on était obligé de déclarer les variables en début de bloc.
Mais, en début de bloc, on ne sait généralement pas ce que doivent valoir ces variables.
Alors, on avait deux choix :

• La performance : On laisse la variable avec une valeur indéterminée et on prie que celui qui maintient le code prenne garde à ne pas lire cette variable tant qu'il ne lui a pas affecté une valeur.
• La robustesse : En attendant de connaître la bonne valeur, on écrit une valeur bidon dans la variable. Comme ça, si celui qui maintient le code lit cette variable par erreur avant de lui avoir affecté la bonne valeur et que cela provoque un bogue en Release, on devrait reproduire le bogue en Debug.

Cependant, dans la version du langage C de 1999, on n'a plus besoin de déclarer les variables en début de bloc.
Du coup, personnellement, je déclare les variables le plus tard possible dans le bloc.
Par exemple, au lieu d'écrire du code de la forme :

Code :

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

1
2
3
4
5
6
{
    int var;
    ...
    var = foo();
    ...
}

j'écris :

Code :

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

1
2
3
4
5
{
    ...
    int var = foo();
    ...
}

ou :

Code :

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

1
2
3
4
5
{
    ...
    const int var = foo();
    ...
}

Les avantages de déclarer les variables le plus tard possible dans le bloc directement avec la bonne valeur sont les suivants :

• Il n'y a pas de dilemme entre la performance et la robustesse.
• Le code est plus concis.
• On peut utiliser la const-correctness. Cela permet parfois de réduire la durée d'analyse du code quand on se demande ce que vaut une variable.
• La portée des variables est plus courte. Cela permet parfois de réduire la durée d'analyse du code quand on se demande à quels endroits une certaine variable est utilisée.

Plusieurs développeurs en C et C++ sont habitués à déclarer les variables en début de bloc et trouvent cela plus esthétique. Mais, de mon point de vue, ce n'est qu'un vestige laissé par une version obsolète du langage C.

[Matt_Houston]

@ l'OP : étant donné que tu génères un .exe j'en ai déduit, apparemment à tort, que tu développais sous Windows natif ou Cygwin / MSYS. Sous Linux, ce ne sont pas les outils de détection d'erreurs qui manquent : valgrind, AddressSanitizer, etc..

[Jipété]

@ l'OP : étant donné que tu génères un .exe j'en ai déduit, apparemment à tort, que tu développais sous Windows natif ou Cygwin / MSYS. Sous Linux, ce ne sont pas les outils de détection d'erreurs qui manquent : valgrind, AddressSanitizer, etc..

Oui, moi aussi ça m'a surpris quand j'ai dézippé et analysé le makefile. Extrait :

Code :

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

all : testBMP.exe testICO.exe testCICO.exe testARAY.exe test.exe

Et puis bon, après correction de quelques bricoles bénignes, ça l'a fait.

Maintenant soyons clair, j'ai juste une curiosité ce truc, on peut voir la même chose avec un éditeur hexa, ou retrouver les mêmes infos en dumpant BitmapFileHeader et BitmatInfoHeader.
Exemple de sortie avec un fichier 8x2x24 en entrée :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
There are 1 images in the file
Doing image number 1
 
Image dimensions: (8,2)
Colors
Row 0 pixels (R,G,B), hex values:
(ff,00,00)(00,ff,ff)(ff,ff,00)(ff,8b,00)(ff,00,ff)(00,80,80)(00,ff,00)(df,df,df)
Row 1 pixels (R,G,B), hex values:
(7f,ff,ff)(7f,7f,ff)(80,00,00)(7f,bf,bf)(bf,bf,7f)(bf,7f,bf)(ff,ff,ff)(00,00,00)
No XOR mask
No AND mask