Discussion :

[Halarp27]

Bonjour, je souhaite libérer une allocation dynamique toute simple avec free, mais celle-ci échoue de manière incompréhensible, la valeur 3 est retournée par le programme.

Mon code :

main.h:

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
#ifndef MAIN_H_INCLUDED
#define MAIN_H_INCLUDED
 
#include <SDL.h>
 
typedef unsigned char Sequence;
 
typedef struct
{
    char quit;
    char key[SDLK_LAST];
    int mousex, mousey;
    int mousexrel, mouseyrel;
    char mousebuttons[8];
}Input;
 
typedef struct
{
    int nbFrames, curentFrame;
    Sequence *SequenceTable;
}AnimationList;
 
typedef struct
{
    SDL_Rect *R;
    int interval, nbAnimations;
    AnimationList *List;
}Animation;
 
typedef struct
{
    SDL_Surface *charset;
    int width, height;
    int nbFramesCharsetX, nbFramesCharsetY;
    Animation Animations;
}Player;
 
#endif // MAIN_H_INCLUDED

main.c:

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
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
#include <string.h>
#include <SDL_image.h>
#include "main.h"
 
SDL_Surface *InitSDL(int width, int height, const char WindowTitle[], const char IconFileName[])
{
    SDL_Surface *screen = NULL;
 
    if(SDL_Init(SDL_INIT_VIDEO) == -1)
    {
        fprintf(stderr, "%s", SDL_GetError());
        exit(-1);
    }
 
    if((screen = SDL_SetVideoMode(width, height, 32, SDL_HWSURFACE | SDL_DOUBLEBUF)) == NULL)
    {
        fprintf(stderr, "%s", SDL_GetError());
        exit(-1);
    }
 
    SDL_WM_SetCaption(WindowTitle, IconFileName);
 
    return screen;
}
 
void UpdateEvents(Input *in)
{
    SDL_Event event;
 
    in->mousebuttons[SDL_BUTTON_WHEELUP] = 0;
    in->mousebuttons[SDL_BUTTON_WHEELDOWN] = 0;
 
    while(SDL_PollEvent(&event))
    {
        switch(event.type)
        {
            case SDL_QUIT:
                in->quit = 1;
                break;
            case SDL_KEYDOWN:
                in->key[event.key.keysym.sym] = 1;
                break;
            case SDL_KEYUP:
                in->key[event.key.keysym.sym] = 0;
                break;
            case SDL_MOUSEMOTION:
                in->mousex = event.motion.x;
                in->mousey = event.motion.y;
                in->mousexrel = event.motion.xrel;
                in->mouseyrel = event.motion.yrel;
                break;
            case SDL_MOUSEBUTTONDOWN:
                in->mousebuttons[event.button.button] = 0;
                break;
            case SDL_MOUSEBUTTONUP:
                if(event.button.button != SDL_BUTTON_WHEELUP && event.button.button != SDL_BUTTON_WHEELDOWN)
                    in->mousebuttons[event.button.button] = 0;
                break;
            default:
                break;
        }
    }
}
 
SDL_Surface *LoadAdaptedIMG(const char IMG_FileName[])
{
    SDL_Surface *IMG = NULL;
    SDL_Surface *IMG_Result = NULL;
 
    if((IMG = IMG_Load(IMG_FileName)) == NULL)
    {
        fprintf(stderr, "%s", IMG_GetError());
        exit(-1);
    }
 
    IMG_Result = SDL_DisplayFormat(IMG);
    SDL_FreeSurface(IMG);
 
    return IMG_Result;
}
 
void LoadCharset(Player *P, FILE *F)
{
    int i, j, nbFrame;
    char charsetName[32] = {0};
 
    fscanf(F, "%s", charsetName);
    P->charset = LoadAdaptedIMG(charsetName);
    fscanf(F, "%d %d", &P->nbFramesCharsetX, &P->nbFramesCharsetY);
    P->Animations.R = malloc(P->nbFramesCharsetX*P->nbFramesCharsetY*sizeof(SDL_Rect));
 
    P->width = P->charset->w / P->nbFramesCharsetX;
    P->height = P->charset->h / P->nbFramesCharsetY;
 
    for(j = 0, nbFrame = 0; j < P->nbFramesCharsetY; j++)
    {
        for(i = 0; i < P->nbFramesCharsetX; i++, nbFrame++)
        {
            P->Animations.R[nbFrame].x = i*P->width;
            P->Animations.R[nbFrame].y = j*P->height;
            P->Animations.R[nbFrame].w = P->width;
            P->Animations.R[nbFrame].h = P->height;
        }
    }
}
 
void GetAnimationInfo(Player *P, FILE *F)
{
    int i;
 
    fscanf(F, "%d %d", &P->Animations.nbAnimations, &P->Animations.interval);
    P->Animations.List = malloc(P->Animations.nbAnimations*sizeof(AnimationList));
    for(i = 0; i <= P->Animations.nbAnimations; i++)
        P->Animations.List[i].SequenceTable = malloc(P->Animations.nbAnimations*sizeof(Sequence));
}
 
void GetAnimationSequences(Player *P, FILE *F)
{
    int i, j;
    int curentAnim;
 
    for(j = 0; j < P->nbFramesCharsetY; j++)
    {
        for(i = 0; i < P->nbFramesCharsetX; i++)
        {
            fscanf(F, "%d", &curentAnim);
            if(curentAnim >= P->nbFramesCharsetX*P->nbFramesCharsetY)
            {
                fprintf(stderr, "Error animation number is out of limit : %d", curentAnim);
                exit(-1);
            }
            P->Animations.List[j].SequenceTable[i] = curentAnim;
 
            fprintf(stderr, "%d ", curentAnim);
        }
 
        fprintf(stderr, "\n");
    }
}
 
Player LoadPlayer(const char PlayerFileName[])
{
    FILE *F = NULL;
    Player P;
    char tag[32] = {0};
 
    if((F = fopen(PlayerFileName, "r")) == NULL)
    {
        fprintf(stderr, "Failled to open %s", PlayerFileName);
        exit(-1);
    }
 
    do
    {
        fscanf(F, "%s", tag);
        if(strstr(tag, "#charset_info"))
            LoadCharset(&P, F);
        if(strstr(tag, "#animation_info"))
            GetAnimationInfo(&P, F);
        if(strstr(tag, "#animation_sequences"))
            GetAnimationSequences(&P, F);
    }while(strstr(tag, "#end") != NULL);
 
    fclose(F);
 
    return P;
}
 
int main(int argc, char* argv[])
{
    int i;
    Input in;
    SDL_Surface *screen = InitSDL(640, 480, "Animation", NULL);
    Player P;
 
    P = LoadPlayer("anim.txt");
    memset(&in, 0, sizeof(in));
 
    SDL_SetColorKey(P.charset, SDL_SRCCOLORKEY, SDL_MapRGB(P.charset->format, 255, 0, 255));
 
    while(!in.quit && !in.key[SDLK_ESCAPE])
    {
        UpdateEvents(&in);
 
        SDL_FillRect(screen, NULL, SDL_MapRGB(screen->format, 255, 255, 255));
        SDL_BlitSurface(P.charset, &P.Animations.R[0], screen, NULL);
 
        SDL_Flip(screen);
        SDL_Delay(5);
    }
 
    free(P.Animations.R);
    for(i = 0; i < P.Animations.nbAnimations; i++)
        free(P.Animations.List[i].SequenceTable);
    free(P.Animations.List);
 
    SDL_FreeSurface(P.charset);
    SDL_Quit();
 
    return 0;
}

L'erreur est à la ligne 195. L'allocation quand à elle se situe à la ligne 112. Je ne comprend vraiment pourquoi ça échoue. Sinon lorsque je fait free(&P.Animations.List), j'ai un warning : warning : attempt to free a non-heap object 'P'

Merci de votre aide

[edgarjacobs]

Bonjour,

Dans la fonction LoadPlayer(....), la variable P est locale. Lorsque on quitte cette fonction, la variable P est détruite.... ainsi que son contenu.

Un solution est de déclarer Player *LoadPlayer(...), et Player *P. Il faut alors allouer P, et le renvoyer. Ce qui sous-entend de modifier également le type de la variable dans le main(...)

[Halarp27]

Dans la fonction LoadPlayer(....), la variable P est locale

Certes, mais elle est retournée à la fin de la fonction.

[edgarjacobs]

Envoyé par Halarp27

Certes, mais elle est retournée à la fin de la fonction.

Lorsque on quitte la fonction LoadPlayer(...), la variable P est détruite.... ainsi que son contenu, c'est-à-dire que les valeurs données aux variables de P (à savoir charset, width, height, nbFramesCharsetX, nbFramesCharsetY, et Animations) sont perdues également.

[Franck.H]

Lorsque on quitte la fonction LoadPlayer(...), la variable P est détruite.... ainsi que son contenu, c'est-à-dire que les valeurs données aux variables de P (à savoir charset, width, height, nbFramesCharsetX, nbFramesCharsetY, et Animations) sont perdues également.

Tu vois, je ne serais pas si sûr, si tu test ce banal code:

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
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
 
 
typedef struct
{
   int nb;
   int lifes;
}Player;
 
Player init_player (void)
{
   Player P;
 
   P.nb = 3;
   P.lifes = 5;
 
   return P;
}
 
int main (void)
{
   Player P = init_player ();
   printf ("Nombre de joueurs : %d\nNombre de vies : %d\n",
           P.nb, P.lifes);
 
   return EXIT_SUCCESS;
}

tu remarqueras qu'à l'exécution on à bien nos données affichées sur la console, le C procède par copie donc il n'y a pas de problème.

Code :

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

1
2
3
Nombre de joueurs : 3
Nombre de vies : 5
hecht@franhec679904:~/Documents/test2/bin/Debug$

Ou bien le comportement diffère d'un système à l'autre ? Après je suis d'accord, ce n'est pas trop dans les pratiques habituelles, surtout les miennes, en générale je procède également par malloc pour les sturcutres

Et d'ailleurs, si cela était le cas et que le problème viens effectivement de là, je pense qu'il aurait eu des segfault bien avant d'arriver aux libérations, surtout sur l'utilisation des images

[edgarjacobs]

Hello,

Lorsqu'une variable locale est créée dans une fonction, elle est créée sur le stack (j'ai toujours confondu les termes 'pile' et 'stack', pour moi le stack est ce qui, lors de l'éxecution du programme, contient les variables créées dynamiquement). Donc, lorsqu'on l'on quitte la fonction, l'adresse de P est retournée, et son contenu peut rester valide un moment (mais le stack, lui est modifié). Mais, pour moi, le contenu de P finira par se faire écraser à un moment ou un autre, lors de nouvelles allocations / créations de variables.

Et si je me trompe (termes ou compréhension), merci de m'éclairer.

Edgar.

[Franck.H]

Oui tu y es presque:

• Stack ou Pile : pour les variables déclarées dans un bloc comme ici
• Heap ou Tas : Pour les allocations dynamique
• Static : Mémoire statique pour les variables déclarées avec le mot-clé static

Après tu as peut-être raison quant à la durée de vie des variables mais étant donné qu'elle n'est pas perdue car retournée par la fonction peut-être que cela change quelque chose, ça je n'en sais pas plus faut avouer

Sinon Halarp27 essaye une autre version de ton code en allouant dynamiquement ta structure principale et en renvoyant un pointeur sur cet espace par le biais de ta fonction d'initialisation, les autres fonctions vu que tu passes un pointeur, tu n'as pas besoin de les modifier à priori. En tout cas c'est de cette manière que j'aurais également fait.

[kwariz]

Bonzour,

tu pourrais être clair plus quant à l'erreur (juste un segfault je suppose ? mais peut-être reçois-tu un message plus complet).
as-tu essayé d'utiliser un outil comme valgrind pour cerner l'erreur (enfin si tu travailles sous linux) ?
Ce serait bien de vérifier les retours des allocations.

Sinon concernant la remarque de EdgarJacobs, oui la variable est «détruite» à la fin de la fonction. Je mets détruite entre guillemet car elle l'est dans le sens «la place est invalide par la suite et sera certainement réutilisée». C'est pourquoi il ne faut jamais retourner l'adresse d'une variable locale. Mais on peut tout à fait retourner la valeur de la variable locale, ici en l'occurrence c'est la valeur d'une structure est copiée ce qui est tout à fait légal.

[Obsidian]

Bonsoir à tous,

L'erreur est à la ligne 195. L'allocation quand à elle se situe à la ligne 112. Je ne comprend vraiment pourquoi ça échoue. Sinon lorsque je fait free(&P.Animations.List), j'ai un warning : warning : attempt to free a non-heap object 'P'

Il y a quand même une chose qui me chagrine : dans LoadPlayer(), tu n'initialises à aucun moment ta variable P avant de l'utiliser et les données membres de celles-ci peuvent prendre n'importe quelles valeurs en fonction de l'état de la mémoire où elles sont déclarées (il faudrait que P soit globale ou statique pour se retrouver dans le .bss et être automatiquement initialisée à zéro).

Donc, si pour une raison ou une autre, ton fichier joueur omet de spécifier un des champs que tu attends, ton programme tournera normalement mais ce champ ne sera jamais initialisé, et tu n'auras aucun moyen de t'en rendre compte.

Il faut donc mettre au préalable tous tes champs à NULL ou à zéro avant de commencer à remplir ta structure. En outre, tu devrais aussi vérifier leur valeur au moment de les libérer. La norme te garantit que free() acceptera silencieusement un NULL sans rien faire, mais rien ne te dit que ce sera aussi le cas avec SDL_FreeSurface().

Bon courage.

[kwariz]

@nanosoft
même si c'est un poil hs
En simplifiant : une struct c'est comme un type de base mais c'est plus gros.
Par exemple sur un code comme :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
int f(int a)
{
  int local=a;
  local=local+1
  return b;
}
 
int main(void)
{
  int x=10;
  int y=f(x);
  printf("x=%d\ny=%d\n", x, y);
  return 0;
}

la ligne y=f(x); provoquera d'abord la création d'une copie de x (sur la pile, dans un registre, ailleurs ... c'est ce qui est définit par l'ABI). Ensuite la fonction f va pouvoir manipuler cette copie. À la fin, la valeur de l'expression donnée au return est copiée quelque part (toujours dépendant de l'ABI) où l'appelant sait qu'il va pouvoir récupérer cette valeur. Cela fait pas mal de copies (parfois) mais comme souvent en C ce sont des type de base qui sont utilisés c'est extrêmement rapide.

Voici le même code avec une structure :

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
struct s_int {
  int value;
};
 
struct s_int f(struct s_int a)
{
  struct s_int local=a;
  local.value=local.value+1;
  return local;
}
 
int main(void)
{
  struct s_int x={10};
  struct s_int y=f(x);
  printf("x=%d\ny=%d\n", x.value, y.value);
  return 0;
}

Et tout se passe de la même manière avec les mêmes copies ... Évidemment ça va prendre plus de temps pour copier une grande structure qu'un entier par exemple ou qu'un pointeur ... c'est une des principales raisons avancée pour éviter le passage par valeur de structure.

[nanosoft]

Merci kwariz,

Cela veut donc dire que le code donné en exemple par Franck est définitivement correct ?

Pour tous : désolé d'insister en limite HS mais j'ai des fonctions qui retournent des structures et je voudrais absolument éclaircir :

Tu vois, je ne serais pas si sûr

Après tu as peut-être raison quant à la durée de vie des variables mais étant donné qu'elle n'est pas perdue car retournée par la fonction peut-être que cela change quelque chose, ça je n'en sais pas plus faut avouer

[kwariz]

Oui il est correct.