Libérer mémoire si erreur allocation

**darkwall_37** · 04/02/2013, 15h47

Bonjour,

J'aurais voulu quelques précisions sur la bonne façon de libérer la mémoire si échec après allocation sur le code suivant.

Notamment pour la fonction

Code :

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

t_file *file_create(char *path, const char *right

file.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
 
#ifndef FILE_H
#define FILE_H
 
typedef struct
{
    FILE *f; //Ptr FILE*
    char *p; //Chemin du fichier
    char *s; //Contenu du fichier dans un tableau de char*
    unsigned long long int l; //Taille du tableau s
}t_file;
 
void  file_close(FILE *f);
FILE *file_open (char *path, const char *r);
t_file *file_create(char *s, const char *r);
char *file_to_string_1(char *p);
char *file_to_string_2(char *p);
char *file_to_string_3( char *p, unsigned long long int *l );
unsigned long long int file_length( char *p );
unsigned long long int file_count_occurrence( char *p, char c );
unsigned long long int file_count_line( char *path );
unsigned long long int *file_asciie_analysis(const char *p);
 
# endif

file.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
 
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
 
#include "file.h"
#include "logger.h"
#include "my_string.h"
#include "allocate.h"
#include "deallocate.h"
 
t_file *file_create(char *s, const char *r )
{
    t_file *f = NULL;
    if ( (f = (t_file *)malloc(sizeof(t_file))) )
    {
            f->f = NULL;
            if ( (f->p = str_copy(s)) )
            {
                if ( (f->s = file_to_string_2(s)) )
                    f->l = file_length(s);
 
                else
                {
                    f->s = NULL;
                    free(f->p);
                    f->p = NULL;
                    free(f);
                    f = NULL;
                }
            }
 
            else
            {
                f->p = NULL;
                free(f);
                f = NULL;
            }
    }
 
    else
    {
        free(f);
        f=NULL,
    }
 
    return f;
}
 
void file_close(FILE *f)
{
    if( fclose(f) != 0 )
        fprintf(logger,"file.h::file_close -> %s\n", strerror(errno));
 
    else fclose(f);
}
 
FILE *file_open(char *p, const char *r)
{
    FILE *f = NULL;
    if ( (f=fopen(p,r)) == NULL )
    {
        fprintf(logger,"file.h::file_open -> %s\n", strerror(errno));
        file_close(f);
        return NULL;
    }
    return f;
}
 
unsigned long long int file_count_line( char *p )
{
    char c;
    unsigned long long int l = 0;
    FILE *f = file_open(p,"r");
 
    while( fscanf(f,"%c",&c) != EOF )
        if (  c == '\n' )
            l++;
 
    file_close(f);
    return l;
}
 
unsigned long long int file_length( char *p )
{
    char c;
    unsigned long long int l = 0;
    FILE *f = file_open(p,"r");
 
    while( fscanf(f,"%c",&c) != EOF )
        l++;
 
    file_close(f);
    return l;
}
 
unsigned long long int file_count_occurrence( char *p, char c )
{
    char *s = file_to_string_2(p);
    unsigned long long int r = str_count_occurrence(s,c);
    free(s);
    return r;
}
 
char *file_to_string_1(char *p)
{
    char c;
    unsigned long long int i = 0;
    char *s = allocate_1D_char( file_length(p)*sizeof(char) );
    FILE *f = file_open(p,"r");
    while( fscanf(f,"%c",&c) != EOF )
    {
        s[i] = c;
        i++;
    }
    file_close(f);
    return s;
}
 
char *file_to_string_2( char *p )
{
    FILE *f=file_open(p,"r");
    char c;
    unsigned long long int l         = 0;
            char *s = allocate_1D_char(1);
            char *t_realloc = NULL;
 
    while( fscanf(f,"%c",&c) != EOF )
    {
        t_realloc = (char *)realloc(s, (l+1)*sizeof(char));
 
        if ( t_realloc )
        {
           s=t_realloc;
           s[l]=c;
           l++;
        }
 
        else
        {
            s=NULL;
            l=0;
        }
    }
 
    file_close(f);
    return s;
}
 
char *file_to_string_3( char *p, unsigned long long int *l )
{
    FILE *f=file_open(p,"r");
    char c;
                 *l = 0;
            char *s = allocate_1D_char(1);
            char *t_malloc = NULL;
 
    while( fscanf(f,"%c",&c) != EOF )
    {
        t_malloc = (char *)realloc(s, (*l+1)*sizeof(char));
 
        if ( t_malloc )
        {
           s=t_malloc;
           s[*l]=c;
           (*l)++;
        }
 
        else
        {
            s=NULL;
            *l=0;
        }
    }
 
    file_close(f);
    return s;
}
 
unsigned long long int *file_asciie_analysis(const char *p)
{
    char ASCIIE_table[256]={'\0','\1','\2','\3','\4','\5','\6','\7','\10','\11','\12','\13','\14','\15','\16','\17','\20','\21','\22','\23','\24','\25','\26','\27','\30','\31','\32','\33','\34','\35','\36','\37','\40','\41','\42','\43','\44','\45','\46','\47','\50','\51','\52','\53','\54','\55','\56','\57','\60','\61','\62','\63','\64','\65','\66','\67','\70','\71','\72','\73','\74','\75','\76','\77','\100','\101','\102','\103','\104','\105','\106','\107','\110','\111','\112','\113','\114','\115','\116','\117','\120','\121','\122','\123','\124','\125','\126','\127','\130','\131','\132','\133','\134','\135','\136','\137','\140','\141','\142','\143','\144','\145','\146','\147','\150','\151','\152','\153','\154','\155','\156','\157','\160','\161','\162','\163','\164','\165','\166','\167','\170','\171','\172','\173','\174','\175','\176','\177','\200','\201','\202','\203','\204','\205','\206','\207','\210','\211','\212','\213','\214','\215','\216','\217','\220','\221','\222','\223','\224','\225','\226','\227','\230','\231','\232','\233','\234','\235','\236','\237','\240','\241','\242','\243','\244','\245','\246','\247','\250','\251','\252','\253','\254','\255','\256','\257','\260','\261','\262','\263','\264','\265','\266','\267','\270','\271','\272','\273','\274','\275','\276','\277','\300','\301','\302','\303','\304','\305','\306','\307','\310','\311','\312','\313','\314','\315','\316','\317','\320','\321','\322','\323','\324','\325','\326','\327','\330','\331','\332','\333','\334','\335','\336','\337','\340','\341','\342','\343','\344','\345','\346','\347','\350','\351','\352','\353','\354','\355','\356','\357','\360','\361','\362','\363','\364','\365','\366','\367','\370','\371','\372','\373','\374','\375','\376','\377'};
    unsigned long long int *l = allocate_1D_ullong(1);
    char *f = file_to_string_3(p,l);
 
    unsigned long long int *stat = allocate_1D_ullong(256);
    unsigned long long int i;
    for (i=0 ; i<*l ; i++)
    {
        if ( f[i] < 0 )
            stat[(unsigned long long int)f[i]*-1+256] += 1;
 
        else stat[(unsigned long long int)f[i]] += 1;
    }
 
    return stat;
}

Merci d'avance.

**Médinoc** · 04/02/2013, 16h56

Bonjour,
Déjà, pourquoi passer path non-constant si la première chose que tu en fais est d'en faire une copie?

**Médinoc** · 04/02/2013, 17h04

Ensuite, je pense que ton nettoyage est un peu redondant. Personnellement, j'aurais plutôt fait ceci (c'est toujours du code boomerang, mais calqué sur le même type de code boomerang utilisé quand on ne retourne pas les données allouées)

Code C :

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
t_file *file_create(char *s, const char *r )
{
	int keep_allocs = 0;
	t_file *f = NULL;
	if ( (f = malloc(sizeof *f)) != NULL )
	{
		const t_file fInit = {0};
		*f = fInit;
 
		if ( (f->p = str_copy(s)) != NULL )
		{
			if ( (f->s = file_to_string_2(s)) != NULL )
			{
				f->l = file_length(s);
				keep_allocs = 1; /*Succès!*/
 
				if(!keep_allocs)
					free(f->s), f->s=NULL;
			}
 
			if(!keep_allocs)
				free(f->p), f->p=NULL;
		}
 
		if(!keep_allocs)
			free(f), f=NULL;
	}
 
	return f;
}

**darkwall_37** · 04/02/2013, 18h05

Ah oui c'est intéressant le coup du keep_alloc.
Vous parlez de boomerang, ce qu'il n'a pas l'air de vous enjouer, y'a t-il un autre moyen de faire sinon ?

Oui c'est vrai, path n'a pas vocation à être modifié, il devrait être de type ptr sur const char.

**Médinoc** · 04/02/2013, 18h31

C'est surtout les autres que ça n'enjoue pas, je trouve qu'en C est difficile à éviter.

En C++ par contre, je serais hostile à ce genre de code.

**Zephyr33** · 11/02/2013, 13h47

Code :

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

1
2
3
4
5
6
7
8
9
if ( (f = (t_file *)malloc(sizeof(t_file))) )
    {
       [...]
    }
else
    {
        free(f);
        f=NULL,
    }

Il ne faut surtout pas free ton pointeur f si l'allocation a échoué, car qui dit échoué dit mémoire non allouée. Donc sans espace alloué, pas d'espace à free ^^
Généralement dans le else { } d'un échec malloc, il vaut mieux que tu affiches un message d'erreur et dans ton cas, que tu retournes NULL

**diogene** · 11/02/2013, 14h07

l ne faut surtout pas free ton pointeur f si l'allocation a échoué, car qui dit échoué dit mémoire non allouée. Donc sans espace alloué, pas d'espace à free ^^

En cas d'échec de l'allocation f == NULL et faire free(NULL) est possible et défini par la norme (comme ne faisant rien).
Le else n'est pas nuisible dans le code, mais inutile.

**Zephyr33** · 12/02/2013, 11h28

Envoyé par diogene

En cas d'échec de l'allocation f == NULL et faire free(NULL) est possible et défini par la norme (comme ne faisant rien).
Le else n'est pas nuisible dans le code, mais inutile.

Ah oui tout à fait, je n'avais pas pensé qu'il pointait sur NULL en écrivant. Ne mettant jamais de free redondant dans mon code, ça m'a fait tiquer

**darkwall_37** · 12/02/2013, 15h56

Envoyé par diogene

En cas d'échec de l'allocation f == NULL et faire free(NULL) est possible et défini par la norme (comme ne faisant rien).
Le else n'est pas nuisible dans le code, mais inutile.

Merci pour les réponses mais comment ça le else est inutile ?

**diogene** · 12/02/2013, 17h51

Le else et son contenu évidemment : le free(f) ne fait rien dans ce cas et f est déjà à NULL.

**gangsoleil** · 13/02/2013, 09h00

Bonjour,

Envoyé par darkwall_37

Merci pour les réponses mais comment ça le else est inutile ?

Regarde bien ce que fait ton code :

Code :

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

1
2
3
4
5
6
7
8
9
if ( (f = (t_file *)malloc(sizeof(t_file))) )
    {
       [...]
    }
else
    {
        free(f);
        f=NULL,
    }

Dans l'ordre :
Appel a malloc pour allouer de la memoire.
Stockage de la valeur de retour de malloc dans f
Test (implicite) si f est NULL ou non
Si f n'est pas NULL, c'est a dire que malloc a pu allouer de la memoire, alors du code
Sinon, c'est a dire si malloc n'a pas pu allouer de memoire et a donc retourne NULL, tu liberes la memoire non allouee, puis tu (re)mets f a NULL. Ces deux operations ne servent a rien. Et comme ton else ne contient rien d'autre, il est strictement inutile.

**darkwall_37** · 13/02/2013, 12h01

Ok, oui, oui en effet. C'est toujours plus clair une fois clairement dit

Si je ne dois pas retourner NULL. Que dois-je faire en cas d’échec de l'allocation pour que cela soit propre ? Sachant que exit, je ne suis pas fan et que sous unix me semble t-il, il faut un return dans le corps même de la fonction sinon un warning.

Sur cette fonction par exemple.

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
unsigned long long int *allocate_1D_ullong( unsigned long long int n )
{
    unsigned long long int i, *p = NULL;
    if ((p = (unsigned long long int *)malloc(n*sizeof(unsigned long long int))))
        for ( i=0 ; i<n ; i++ )
            p[i]=0;
 
    else fprintf(logger, "allocate.h::allocate_1D_ullong -> %s\n", strerror(errno));
 
    return p;
}

Merci

**Médinoc** · 13/02/2013, 12h05

Je trouve que retourner NULL est tout à fait acceptable, surtout en C où l'on n'a pas la gestion d'exceptions pour remonter ce genre d'erreurs.

**darkwall_37** · 13/02/2013, 12h17

Ok, donc tel quel, ça reste correcte. Je vais m'y tenir alors.

**Médinoc** · 13/02/2013, 13h21

Note toutefois qu'on n'est pas censé caster le retour de malloc() en C. C'est même potentiellement nuisible car ça masque un éventuel warning en cas d'oubli d'inclusion de <stdlib.h> (ce qui serait nuisible sur la plupart des plate-formes 64 bits, ainsi que sur d'autres plate-formes plus ésotériques comme une architecture 68k)

**darkwall_37** · 13/02/2013, 13h48

Sous unix, je récupère des warnings si le malloc n'est pas casté de mémoire c'est pourquoi je cast les malloc depuis. Mais peut être que je me trompe étant donné que c'est de mémoire, et j'ai une très mauvaise mémoire.

**Médinoc** · 13/02/2013, 14h43

Tu dois te tromper. Avec un compilateur moderne, les seules sources de warnings causés par l'absence de cast de malloc() sont:

oubli d'inclusion de <stdlib.h> (malloc() est donc implicitement déclarée comme retournant un int)
compilation en C++ au lieu de C (vérifie que l'extension de ton fichier source est .c et non .C, c'est une cause commune de compilation C++ sous *n*x)

**Zephyr33** · 13/02/2013, 19h30

Envoyé par darkwall_37

Sous unix, je récupère des warnings si le malloc n'est pas casté de mémoire c'est pourquoi je cast les malloc depuis. Mais peut être que je me trompe étant donné que c'est de mémoire, et j'ai une très mauvaise mémoire.

Malloc retourne un void * tu n'a aucune besoin de cast. La valeur retournée est l'adresse du premier octet de la zone mémoire allouée. Donc un pointeur qui pointe déjà sur un type *.

**Sve@r** · 17/02/2013, 10h43

Envoyé par darkwall_37

Sous unix, je récupère des warnings si le malloc n'est pas casté de mémoire

Bonjour
Non, ton warning vient (probablement) du fait que tu n'inclues pas stdlib.h. Le compilo n'ayant pas le type de malloc (void *) le considere donc de-facto comme int lorsqu'il le rencontre dans ton code. Ensuite tu copies son retour (vu comme int) dans un pointeur donc warning du compilo.

Envoyé par darkwall_37

c'est pourquoi je cast les malloc depuis.

Mauvaise analyse de la cause donc mauvaise solution. A une epoque lointaine le type void * n'existant pas, malloc etait du type char * donc cast obligatoire. Mais aujourd'hui ce n'est plus necessaire voire meme comme le dit Medinoc cela peut etre nefaste. Toutefois si certains (comme Souviron34) defendent avec de tres bons arguments le cast de malloc (http://www.developpez.net/forums/d85...ction-malloc-c) ; aucun ne caste pour "masquer" un warning dont il n'avait pas compris l'origine...

Envoyé par darkwall_37

Mais peut être que je me trompe étant donné que c'est de mémoire, et j'ai une très mauvaise mémoire.

C'est pas grave. Maintenant tu sais

**darkwall_37** · 21/02/2013, 10h04

Okay alors adieu le cast sur malloc

J'avais une question subsidiaire, notamment concernant l'allocation de tableaux à deux dimensions.

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
 
int **allocate_2D_int( unsigned long long int x, unsigned long long int y )
{
    int **t = NULL;
    if ( (t = malloc( x*sizeof(int*))) )
    {
        int *test_malloc=NULL;
        unsigned long long int i;
        for ( i=0 ; i<x ; i++ )
        {
            if ( (test_malloc = allocate_1D_int(y)) )
                t[i] = test_malloc;
 
            else 
            {
                unsigned long long int j = i;
                for (i=0 ; i<j ; i++)
                    free(t[i])
 
                free(t);
            }
        }
    }
    return t;
}

Est-il utile de tester chaque allocation dans le for ? Ce que je ne fais jamais d'habitude. Mais là je me disais quand même, le faut-il ou pas ?

Merci bien

Libérer mémoire si erreur allocation

C

Discussions similaires

Partager

Partager