Discussion :

[Fooshi]

bonjour,
je souhaite remplacer plusieurs lignes de codes par une macro :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
	if (msg)
		{
			msg[msg_len] = '\0';
			break;
		}
		msg_len++;
		msg = (char *) malloc(msg_len);
		if (check_not(msg != NULL, "Can't alloc msg"))
		{
			return -1;
		}

ci dessous ma macro qui la remplace :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
#define END_MSG(msg, msg_len) \
	do { \
		if (msg) \
		{ \
			msg[msg_len] = '\0'; \
			break; \
	    } \
		msg_len++; \
		msg = (char *) malloc(msg_len); \
		if (check_not(msg != NULL, "Can't alloc msg")) \
		{ \
			return -1; \
		} \
	} while(0)

la macro me donne un problème , autrement dis l'allocation mémoire ne se fais pas ! (j'ai bien essaye if (msg == NULL) ... )

ma macro est t'elle mal écrite ?
Merci d'avance.

[jowo]

Bonjour,

Pour les macros, la bonne pratique consiste à entourer par des parenthèses les paramètres de la macro.

Mais attention aux cas particuliers. Il me semble que tu les a cumulée dans ta macro.

Une bonne macro est une macro courte.

Ton return dans une macro ne me plait pas beaucoup.

Comment utiles-tu la macro?

Pourquoi ne pas écrire une fonction?

[Médinoc]

Fais gaffe, le do-while a changé la signification du break...

[Fooshi]

Ca doit etre ca, je vais ecrire une fonction. Merci quand meme

[jabbounet]

Ce que je vois en indentant ta macro

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
 
#define END_MSG(msg, msg_len)                          \
     do {                                             \
       if (msg)                                       \
         {                                            \
           msg[msg_len] = '\0';                       \
           break;                                     \
         }                                            \
       msg_len++;                                     \
       msg = (char *) malloc(msg_len);                \ <--- Risque de memory leak car tu perd la référence précendent à msg.
       if (check_not(msg != NULL, "Can't alloc msg")) \
         {                                            \
           return -1;                                 \
         }                                            \
     } while(0)

C'est que ton malloc est fait de facon systematique que msg soit NULL ou non.
tu as un risque de memory leak ici.

malloc et les macro n'ont pas de soucis particulier.
j'ai fait le test suivant et ça marche.

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>
 
#define CMSG(msg, msg_len)                                  \
  if (msg == NULL) {                                        \
    msg = malloc(msg_len+1);                                  \
  }                                                         \
  if (msg == NULL) {                                        \
    printf("Erreur d'allocation\n");                        \
  }                                                         \
  for(i=0;i<msg_len;i++) {                                  \
    msg[i] = 'a' + i ;                                      \
  }                                                         \
  msg[msg_len] = '\0';      
 
 
 
 
int main (int argc, char ** argv) {
  int i = 0;
  char * wasa = NULL;
  char * toto = NULL;
  CMSG(wasa,17);
  CMSG(toto,5);
  printf("%s - %s\n", wasa , toto);
 
 
  return 0;
}

-*- mode: compilation; default-directory: "c:/" -*-
Compilation started at Thu Jul 01 16:36:59

gcc toto.c -o toto && toto
abcdefghijklmnopq - abcde

Compilation finished at Thu Jul 01 16:37:00

[gl]

Ce que je vois en indentant ta macro

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
 
#define END_MSG(msg, msg_len)                          \
     do {                                             \
       if (msg)                                       \
         {                                            \
           msg[msg_len] = '\0';                       \
           break;                                     \
         }                                            \
       msg_len++;                                     \
       msg = (char *) malloc(msg_len);                \ <--- Risque de memory leak car tu perd la référence précendent à msg.
       if (check_not(msg != NULL, "Can't alloc msg")) \
         {                                            \
           return -1;                                 \
         }                                            \
     } while(0)

C'est que ton malloc est fait de facon systematique que msg soit NULL ou non.
tu as un risque de memory leak ici.

Non, malloc() n'est pas fait de façon systématique ici. Si msg n'est pas NULL, on sort de la boucle (et donc malloc() n'est pas exécuté) avec le break.

[jabbounet]

Non, malloc() n'est pas fait de façon systématique ici. Si msg n'est pas NULL, on sort de la boucle (et donc malloc() n'est pas exécuté) avec le break.

oups j'avais de la #ù$**** dans les yeux.....