Discussion :

[ternel]

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

[darkwall_37]

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 ?

[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).

[ternel]

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!

[gangsoleil]

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 :

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

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 :

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

[Bktero]

Code :

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

unsigned long long int i;

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

[darkwall_37]

@gangsoleil

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

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

[gangsoleil]

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 :

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

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

[darkwall_37]

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

[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.

[darkwall_37]

• 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.

[Sve@r]

Okay alors adieu le cast sur malloc

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

@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...

@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...

[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 ?

[gangsoleil]

@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 ?

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

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

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.

[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."

[gangsoleil]

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

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]

[darkwall_37]

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

[darkwall_37]

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]

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