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

**darkwall_37** · 22/02/2013, 16h27

Envoyé par Médinoc

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;
}

J'oubliais ! je ne comprends pas cette partie de votre code :

Code :

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

1
2
3
 
const t_file fInit = {0};
*f = fInit;

Cdt.

**Médinoc** · 22/02/2013, 16h44

La ligne const t_file fInit = {0}; crée une structure t_tfile avec tous les champs à zéro de manière portable (sur certaines plate-formes, zéro ne signifie pas forcément "tous les bits à zéro")
La ligne suivante *f = fInit; recopie ça dans la structure allouée.

En gros, c'est une version plus portable de memset(f, 0, sizeof(*f)), ou d'un appel à calloc().

Edit: Pour bien faire, on pourrait mettre ça dans une ou deux fonctions:

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
void file_init0(t_file *f)
{
	const t_file fInit = {0};
	*f = fInit;
}
 
t_file* file_new0()
{
	t_file *f = malloc(sizeof *f);
	if(f != NULL)
		file_init(f);
	return f;
}
 
...
 
/*Dans la fonction file_create()*/
	if ( (f = file_new0()) != NULL )
	{

Libérer mémoire si erreur allocation

C

Vue hybride

Discussions similaires

Partager

Partager