Discussion :

[Freed0]

Salut

Bon voilà, j'aimerais bien quelques conseils de votre part...

Tout d'abord, j'aimerais savoir comment bien utiliser les outils permettant de sortir d'une fonction, j'entend par là return et exit().

Pour le moment, je travaille de cette manière :

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
ma_fonction_exemple(void)
{
    FILE *fp = NULL;
    /* Valeur de retour */
    int ret_value = EXIT_SUCCESS;
 
    /* Appels de fonctions, etc. Par exemple : */
    fp = fopen("monfichier.dat", "rb");
 
    if(fp != NULL)
    {
        /* Traitement */
 
        fclose(fp);
    }
    else
    {
        fprintf(stderr, "Erreur !");
        ret_value = EXIT_FAILURE;
    }
 
    return ret_value;
}

Le seul inconvénient que je trouve actuellement à cette méthode est que l'on s'y perd assez rapidement, et que ça peut devenir assez pénible à décoder dans certains cas...

Parcontre il m'est souvent arrivé de lire ceci :

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
int
ma_fonction_exemple(void)
{
    FILE *fp = NULL;
 
    /* Appels de fonctions, etc. Par exemple : */
    fp = fopen("monfichier.dat", "rb");
 
    if(fp == NULL)
    {
        fprintf(stderr, "Erreur !");
        return EXIT_FAILURE;
        /* Je retrouve aussi la forme suivante :
        exit(-1);
        */
    }
 
    /* Traitement */
 
    fclose(fp);
 
    return EXIT_SUCCESS;
}

Est-ce que cette dernière méthode est conseillée/propre ? L'utilisez-vous ? Si oui, pourquoi ?


Ensuite, j'aimerais votre avis sur la fonction suivante :

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
int
get_hw_addr(struct ether_addr *mac, char *s)
{
    char    *p = NULL;
    char    delim[] = ":";
    int     ret_value = EXIT_SUCCESS;
    int     i = 0;
 
    /* Premier octet */ 
    p = strtok(s, delim);
 
    if(p != NULL)
        mac->octet[i] = (u_char)strtol(p, NULL, 16);
    else
        ret_value = EXIT_FAILURE;
 
    i++;
 
    /* 5 octets suivants */
    while(i < 6 && p != NULL)
    {
        p = strtok(NULL, delim);
 
        if(p != NULL)
            mac->octet[i] = (u_char)strtol(p, NULL, 16);
        else
            ret_value = EXIT_FAILURE;
 
        i++;
    }
 
    return ret_value;
}

C'est une fonction qui découpe la chaine de caractère fournie en paramètre en adresse ethernet. Ce résultat est stocké sous la forme d'une struct ether_addr (dont j'ai imité la forme, car je travaille sous Windows).

Pour le moment, j'ai quelques problèmes : je ne sais pas comment repérer toutes les erreurs qui pourraient intervenir... Par exemple si on utilise une lettre ne faisait pas partie de l'intervalle hexadécimal, strtol renvoie 0, et donc mon adresse ethernet comportera l'octet 0, ce qui n'était pas voulu...

Je voudrais aussi savoir si mon utilisation de strtok est correcte ?


Merci d'avance pour vos conseils

Fred

[souviron34]

pour le if ( .. == NULL ) else..

en gros les 2 formes sont absolument corrrectes..

Tout dépend du volume de code et du volume de la fonction.... En fait c'est beaucoup plus pour faciliter la lecture/maintenance qu'autre chose.

Donc, suivant les cas, c'est l'un ou l'autre...

Dans le cas que tu présentes, j'aurais tendance à préférer la 2ième solution, car il peut y avoir beaucoup de code de traitement, et donc avoir tout un grand paragraphe indenté, avec éventuellement des indendations à l'intérieur, est plus complexe à lire.

En ce qui concerne le problème exit/return, là encore ça dépend de l'utilisation. Pour un programme devant être "safe-proof", c'est à dire ne quittant jamais, il est évident que le return est à utiliser, avec traitements appropriés en sortie (même si c'est qu'une allocation de mémoire n'a pu se faire). Par contre, dans le cas de programmes généraux, ne nécessitant pas une robustesse de running-time, le mieux est de faire des exit lorsqu'on rencontre une erreur du style fatal (allocation échoue, fichier non trouvé, etc..).


Enfin, en ce qui concerne ton 3ième problème, je rajouterais à ce compte-là dans le if ( p != NULL ) une boucle testant sur la longueur de p la validité des caractères, avant de faire le strtol...

[Skyounet]

Certaines personnes ici sont partisanes du "un seul return par fonction"...

Moi je prefère plusieurs return, parce que sinon je trouve que l'on accumule les imbrications (if(* != NULL)...) et ça devient vite illisible.

Juste un avis personnel.

[Emmanuel Delahaye]

Pour le moment, je travaille de cette manière :

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
ma_fonction_exemple(void)
{
    FILE *fp = NULL;
    /* Valeur de retour */
    int ret_value = EXIT_SUCCESS;
 
    /* Appels de fonctions, etc. Par exemple : */
    fp = fopen("monfichier.dat", "rb");
 
    if(fp != NULL)
    {
        /* Traitement */
 
        fclose(fp);
    }
    else
    {
        fprintf(stderr, "Erreur !");
        ret_value = EXIT_FAILURE;
    }
 
    return ret_value;
}

Le seul inconvénient que je trouve actuellement à cette méthode est que l'on s'y perd assez rapidement, et que ça peut devenir assez pénible à décoder dans certains cas...

Personnellement j'écris ça comme ceci :

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
 
int ma_fonction_exemple (void)
{
   int err = 0;
   FILE *fp = fopen ("monfichier.dat", "rb");
 
   if (fp != NULL)
   {
      /* Traitement */
 
      fclose (fp), fp = NULL;
   }
   else
   {
      fprintf (stderr, "Erreur !\n");
      err = 1;
   }
 
   return err;
}

ce qui est plus clair à mon sens...

Je n'ai, en principe, pas de bonne raison de faire autrement.

[Emmanuel Delahaye]

Je voudrais aussi savoir si mon utilisation de strtok est correcte ?

Pas besoin de strtok() :

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
 
#include <stdlib.h>
#include <stdio.h>
#include <assert.h>
 
struct ether_addr
{
   unsigned char octet[6];
};
 
int get_hw_addr (struct ether_addr *mac, char const *s)
{
   int err = 0;
   char const *pend = s;
   int delim = ':';
   size_t i = 0;
 
   while (!err && *pend != 0 && i < sizeof mac->octet / sizeof *mac->octet)
   {
      /* ce cast est du a une incoherence dans la specification de strtoul(). */
      unsigned n = strtoul (pend, (char **) &pend, 16);
 
      err = *pend != delim && *pend != 0;
 
      if (!err)
      {
         pend++;
         err = n > 255;
         if (!err)
         {
            mac->octet[i] = n;
            i++;
         }
      }
   }
   return err;
}
 
int main (void)
{
 
   struct ether_addr mac;
   int err = get_hw_addr (&mac, "12:34:56:78:9A:BC");
 
   if (!err)
   {
      size_t i;
      for (i = 0; i < sizeof mac.octet / sizeof *mac.octet; i++)
      {
         if (i == 0)
            printf ("%02X", mac.octet[i]);
         else
            printf (":%02X", mac.octet[i]);
 
      }
      printf ("\n");
   }
   else
      puts ("error");
   return 0;
}

[Médinoc]

exit() ne sort pas de la fonction courante, mais de tout le processus, et selon moi ne peut être utilisée correctement (surtout en C++, même si là on est en C).

return sort assez proprement de la fonction courante, et le problème est plus de l'utiliser lisiblement, en C comme en C++.

[Freed0]

Bon, je détère mon topic. En passant, merci pour vos réponses (j'avais pas pensé à revenir poster ).

Donc j'aimerais juste votre avis sur ce warning :

Code :

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

1
2
3
wol.c: In function 'main':
wol.c:29: warning: missing initializer
wol.c:29: warning: (near initialization for 'to.sin_port')

Voici la ligne concernée :

Code :

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

1
2
3
28:  struct ether_addr    ether = {{0}};
29:  struct sockaddr_in   to = {0};
30:  unsigned char        packet[102] = {0};

Et enfin les options utilisées pour la compilation (chippées chez Emmanuel) :

Code :

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

gcc -Wall -Wextra -O2 -Wwrite-strings -Wstrict-prototypes

Ensuite, j'aimerais votre avis sur ce code. Toutes les remarques (fondées) sont les bienvenues. Problèmes, améliorations intéressantes, etc.
Voici le fichier en question : http://myspace.voo.be/tjacquelin/src/wol.c

Merci

[Emmanuel Delahaye]

Donc j'aimerais juste votre avis sur ce warning :

Code :

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

1
2
3
wol.c: In function 'main':
wol.c:29: warning: missing initializer
wol.c:29: warning: (near initialization for 'to.sin_port')

Tu peux ignorer.

[Freed0]

Tu peux ignorer.

Je suppose que je peux aussi enlever les doubles accolades de la ligne 28 alors ? Elle génèrent le même type de warning...