Libérer mémoire si erreur allocation

Version imprimable

Voir 40 message(s) de cette discussion en une page

21/02/2013, 10h10
ternel

il faut tester et libérer chaque allocation.
tester pour éviter les "segmentation faults", libérer pour éviter les fuites mémoires.
21/02/2013, 10h51
darkwall_37

Citation:

Envoyé par leternel

il faut tester et libérer chaque allocation.
tester pour éviter les "segmentation faults", libérer pour éviter les fuites mémoires.

Et de ce fait, le code précédent est-il correcte à vos yeux ?
21/02/2013, 11h19
Médinoc

L'alternative, c'est de faire moins d'allocations, ce qui a l'avantage de garder ton tableau sous forme d'un bloc continu (enfin, deux blocs, si on compte celui des pointeurs).
21/02/2013, 11h27
ternel

Citation:

Envoyé par darkwall_37

Et de ce fait, le code précédent est-il correcte à vos yeux ?

non, parce que si une erreur d'allocation survient, tout ce qui a été alloué est désalloué (par un free() direct, et non un appel à un desallocation_1D()…) mais la boucle continue!

Citation:

Envoyé par darkwall_37

Et de ce fait, le code précédent est-il correcte à vos yeux ?

Correcte probablement, mais tres complexe pour pas grand chose.
En essayant de simplifier (attention, code non teste)

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
unsigned long long int *allocate_1D_ullong( unsigned long long int n )
{
    unsigned long long *p;
 
    errno = 0;
    p = calloc(n*sizeof(unsigned long long int), 1); 
 
    if (p == NULL)
    {
      fprintf(logger, "allocate.h::allocate_1D_ullong -> %s\n", strerror(errno));
    }
 
    return p;
}

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
 
int **allocate_2D_int( unsigned long long int x, unsigned long long int y )
{
    int **t;
    if ( (t = malloc( x*sizeof(int*))) )
    {
        int *test_malloc;
        unsigned long long int i;
 
        for ( i=0 ; i<x ; i++ )
        {
            if ( (t[i] != allocate_1D_int(y)) )
            {                
                unsigned long long int j ;
                for (j=i ; j>0 ; i--)
                {
                    free(t[j]) , t[j] = NULL;
                }
                free (t), t = NULL;
            }
        }
    }
    return t;
}

21/02/2013, 11h40
Bktero

Code:

unsigned long long int i;

Le type size_t devrait être utilisé pour parcourir des tableaux.
Voir http://www.viva64.com/en/t/0044/

@gangsoleil

Code:

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;
    if ( (t = malloc( x*sizeof(int*))) )
    {
        int *test_malloc; // On peut le virer ?
        unsigned long long int i;
 
        for ( i=0 ; i<x ; i++ )
        {
            t[i]=? avant de le tester
            if ( (t[i] != allocate_1D_int(y)) )
            {                
                unsigned long long int j ;
                for (j=i ; j>0 ; i--)
                {
                    free(t[j]) , t[j] = NULL;
                }
                free (t), t = NULL;
            }
        }
    }
    return t;
}

Cordialement

21/02/2013, 11h50
Médinoc

Je crois que tu voulais dire if ( !(t[i] = allocate_1D_int(y)) )
Ou en plus clair: if ( (t[i] = allocate_1D_int(y)) == NULL )
21/02/2013, 11h54
gangsoleil
Citation:

Envoyé par Médinoc

Je crois que tu voulais dire if ( !(t[i] = allocate_1D_int(y)) )
Ou en plus clair: if ( (t[i] = allocate_1D_int(y)) == NULL )

Oui, bien sur.

J'ai tellement l'habitude d'ecrire tout sur plusieurs lignes que j'ai "corrige" t[i] = alloc() en t[i] != alloc()
Je reprends donc :
Code:

1 2 3 4 5 for ( i=0 ; i<x ; i++ ) { t[i] = allocate_1D_int(y); if ( t[i] == NULL )

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
int **allocate_2D_i( unsigned long long int x, unsigned long long int y )
{
    int **t = NULL;
    if ( (t = malloc( x*sizeof(int*))) )
    {
        unsigned long long int i;
        for ( i=0 ; i<x ; i++ )
        {
            if ( !( t[i] = allocate_1D_i(y) ) )
            {
                unsigned long long int j;
                for (j=i-1 ; 0<=j ; j--) // j= i-1 car l'allocation à j=i à echouée donc pas besoin de desallouer et il faut aller jusqu'à 0 non ?
                    free(t[j]), t[j]=NULL;
 
                free(t), t=NULL;
                i=x;
            }
        }
    }
    else fprintf(logger,"allocate.h::allocate_2D_i -> %s\n", strerror(errno));
    return t;
}

Pour la boucle en effet, j'avais oublier "i=x" pour sortir de la boucle en cas d'erreur.

@gangsoleil, je vois que vous mettez errno à 0 chaque fois qu'errno est susceptible de changer de valeur suite à une erreur. Il faut donc que j'initialise mon errno à 0 à chaque début de fonction où une erreur est susceptible de se produire ?
@Medinoc, ce que vous faites c'est comme un tableau 1D que vous transformez en 2D ? J'ai pas tout compris =)

21/02/2013, 12h12
Médinoc
- Ton alloue X tableaux de Y entiers (représentant chacun une colonne), et tableau de X pointeurs pointant sur chaque tableau.
- Mon code fait un tableau de X*Y entiers, et un tableau de X pointeurs pointant sur chaque "colonne" du tableau.
Dans les deux cas, à la fin on a l'équivalent d'un tableau 2D de X*Y entiers, et on s'en sert de la même façon: tab[x][y] à travers deux niveaux de pointeurs.
21/02/2013, 12h19
darkwall_37
Citation:
Envoyé par Médinoc

Ton alloue X tableaux de Y entiers (représentant chacun une colonne), et tableau de X pointeurs pointant sur chaque tableau.
Mon code fait un tableau de X*Y entiers, et un tableau de X pointeurs pointant sur chaque "colonne" du tableau.

Dans les deux cas, à la fin on a l'équivalent d'un tableau 2D de X*Y entiers, et on s'en sert de la même façon: tab[x][y] à travers deux niveaux de pointeurs.
Ah okay, intéressant en effet. Il faut que je regarde ça de plus près.
21/02/2013, 13h16
Sve@r

Citation:

Envoyé par darkwall_37

Okay alors adieu le cast sur malloc :cry:

Pas forcement. Si tu as des raisons personnelles que tu juges bonnes, si tu n'as pas de warning lors de tes malloc sans cast alors tu pourras recommencer le cast mais cette fois pour de bonnes raisons (les tiennes) et non pour eviter les warning. Un warning n'est d'ailleurs jamais une tres bonne chose. Il signifie "je (le compilo) ne suis pas certain d'avoir bien compris ce que vous voulez faire". Dans ces conditions il vaut mieux le resoudre que le masquer car cela elimine alors 50% des bugs potentiels :P

Citation:

Envoyé par darkwall_37

@gangsoleil, je vois que vous mettez errno à 0 chaque fois qu'errno est susceptible de changer de valeur suite à une erreur. Il faut donc que j'initialise mon errno à 0 à chaque début de fonction où une erreur est susceptible de se produire ?

Bonjour
Dans l'absolu ce n'est pas utile. Si on regarde errno apres que l'erreur se soit produite, qu'il ait ete a zero ou pas avant l'erreur importe peu.
Cela ne se justifiera que si tu fais un traitement pointu sur plusieurs etapes et que tu veux verifier que errno ne bouge pas durant toutes les etapes...

Citation:

Envoyé par darkwall_37

@Medinoc, ce que vous faites c'est comme un tableau 1D que vous transformez en 2D ? J'ai pas tout compris =)

Regarde un echiquier. Chaque case est identifie par une position en [lig][col] mais si tu le decoupes et le mets bout a bout, alors chaque meme case pourra etre identifiee par un unique index [i]
D'une facon generale, une case [x][y] d'un tableau 2D peut aussi etre identifiee par l'index [x * nb_cols + y]. D'ailleurs la memoire d'un ordi n'est qu'en une seule dimension... ;)
21/02/2013, 13h24
darkwall_37

Oui je vois, je suis familiarisé avec les échecs, j'en ai fait 15 ans en clubs =).
Pour les tableaux 2D->1D, je connais bien. En VBA notamment, je les utilise parce qu'il n'est pas possible de redimensionner la taille des colonnes.
D'ailleurs, est-ce plus performant d'utiliser un tableau 1D comme tableau 2D bien qu'il y ait la fonction de transformation d'indice 2D -> 1D ?

Citation:

Envoyé par darkwall_37

@gangsoleil, je vois que vous mettez errno à 0 chaque fois qu'errno est susceptible de changer de valeur suite à une erreur. Il faut donc que j'initialise mon errno à 0 à chaque début de fonction où une erreur est susceptible de se produire ?

Citation:

Envoyé par Sve@r

Dans l'absolu ce n'est pas utile. Si on regarde errno apres que l'erreur se soit produite, qu'il ait ete a zero ou pas avant l'erreur importe peu.
Cela ne se justifiera que si tu fais un traitement pointu sur plusieurs etapes et que tu veux verifier que errno ne bouge pas durant toutes les etapes...

En fait, la norme precise que les fonctions peuvent mettre errno a une valeur, pas qu'elles doivent le faire.
Et l'experience m'a montre que dans je-ne-sais-plus-quel-cas, c'est effectivement le cas, ce qui fait qu'on peut se retrouver avec une ancienne valeur d'errno.

Exemple avec le code suivant, en supposant que les deux fonctions appelees sont en erreur (c'est a dire qu'elles retournent -1), que la premiere met errno a quelque chose, et que la seconde ne fait pas son boulot sur errno :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 ret_code = appel_fontion (param, param2); if (ret_code == -1) { /* traitement d'erreur, mais qui ne quitte pas */ } ret_code = appel_fonction_qui_n_affecte_pas_errno (param3); if (ret_code == -1) { printf ("erreur : %d, %s\n", errno, strerror(errno)); } /* suite du code */
J'ai passe des heures a comprendre pourquoi "appel_fonction_qui_n_affecte_pas_errno" plantait comme elle le disait, mais elle plantait pour une autre raison en fait, et ne mettait pas errno.

Depuis, je mets errno a 0 avant tout appel a une fonction.

21/02/2013, 15h23
Médinoc

J'ai déjà eu le même genre de problème avec GetLastError(). C'est de telles fonctions que proviennent les erreurs "L'opération s'est terminée avec succès."
21/02/2013, 15h39
gangsoleil

[HS, mais presque dans le sujet, mais pas vraiment non plus]

Citation:

Envoyé par Médinoc

C'est de telles fonctions que proviennent les erreurs "L'opération s'est terminée avec succès."

Attention, certaines API ont des codes de retour "ERROR_SUCCESS"

[fin du HS]
22/02/2013, 09h02
darkwall_37

Merci pour votre aide et vos réponses aussi claires que précise. Sujet clos. Pour le moment :mrgreen:

Citation:
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:

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:

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

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:

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 ) {

Voir 40 message(s) de cette discussion en une page