Nous vous invitons à essayer de trouver une erreur dans une fonction très simple du projet GNU Midnight Commander. Pourquoi? Comme ça. C'est drôle et intéressant. Bien que non, nous avons menti. Encore une fois, nous voulons démontrer une erreur qu'une personne trouve avec difficulté dans le processus de révision du code, mais trouve facilement l'analyseur de code statique PVS-Studio.
Récemment, nous avons reçu une lettre demandant pourquoi l'analyseur génère un avertissement sur la fonction
EatWhitespace , dont le code est donné ci-dessous. En fait, la question n'est pas si simple. Essayez de découvrir vous-même ce qui ne va pas avec ce code.
static int EatWhitespace (FILE * InFile) { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); }
Comme vous pouvez le voir, la fonction
EatWhitespace est très petite. Même un commentaire sur une fonction prend plus de place que le corps de la fonction elle-même :). Maintenant quelques détails.
Description de la fonction
Getc :
int getc ( FILE * stream );
La fonction renvoie le caractère pointé par l'indicateur interne de la position du fichier du flux spécifié. L'indicateur passe ensuite au caractère suivant. Si la fin du fichier est atteinte au moment de l'appel au flux, la fonction renvoie
EOF et définit l'indicateur de fin de fichier pour ce flux. Si une erreur de lecture se produit, la fonction renvoie une valeur EOF et définit un indicateur d'erreur pour le flux donné (ferror).
Description de la fonction
isspace :
int isspace( int ch );
La fonction vérifie si le caractère est un espace selon la classification des paramètres régionaux actuels. Dans les paramètres régionaux standard, les caractères suivants sont des espaces blancs:
- espace (0x20, ``);
- changement de page (0x0c, '\ f');
- saut de ligne LF (0x0a, '\ n');
- retour chariot CR (0x0d, '\ r');
- onglet horizontal (0x09, '\ t');
- onglet vertical (0x0b, '\ v').
Valeur de retour Valeur différente de zéro, si le caractère est un espace, zéro sinon.
La fonction
EatWhitespace doit ignorer tous les caractères considérés comme des espaces, à l'exception du saut de ligne '\ n'. Une autre raison pour arrêter la lecture d'un fichier peut être la fin du fichier (EOF).
Et maintenant, sachant tout cela, essayez de trouver une erreur!
Pour éviter que le lecteur ne regarde accidentellement immédiatement la réponse, ajoutez quelques licornes en attente.
Figure 1. Il est temps de rechercher une erreur. Les licornes attendront.Vous ne voyez toujours pas l'erreur?
Le fait est que nous avons trompé les lecteurs sur
isspace . Haha Ce n'est pas du tout une fonctionnalité standard, mais une macro maison. Oui, nous sommes irréprochables et vous a rendu confus.
Figure 2. Une licorne donne aux lecteurs une fausse impression de ce qu'est l' espace .En fait, bien sûr, nous et notre licorne ne sommes pas à blâmer. Les auteurs du projet GNU Midnight Commander ont contribué à la confusion en décidant de créer leur propre implémentation
isspace dans le fichier
charset.h :
#ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t')
En créant une telle macro, certains développeurs ont confondu d'autres développeurs. Le code est écrit en supposant que
isspace est une fonction standard qui considère les retours chariot (0x0d, '\ r') comme l'un des caractères d'espacement.
La macro implémentée considère uniquement les espaces et les tabulations comme des espaces. Remplaçons la macro et voyons ce qui se passe.
for (c = getc (InFile); ((c)==' ' || (c) == '\t') && ('\n' != c); c = getc (InFile))
La sous-expression ('\ n'! = C) est redondante (redondante) car son résultat sera toujours vrai. L'analyseur PVS-Studio met en garde à ce sujet, donnant un avertissement:
V560 Une partie de l'expression conditionnelle est toujours vraie: ('\ n'! = C). params.c 136.
Pour plus de clarté, analysons 3 options pour le développement d'événements:
- La fin du fichier est atteinte. La fin de fichier (EOF) n'est pas un espace ou un onglet. La sous-expression ('\ n'! = C) n'est pas calculée en raison d' une évaluation de court-circuit . Le cycle s'arrête.
- Tout caractère qui n'est ni un espace ni une tabulation est lu. La sous-expression ('\ n'! = C) n'est pas calculée en raison d'une évaluation de court-circuit. Le cycle s'arrête.
- Lisez un caractère d'espace ou un onglet horizontal. La sous-expression ('\ n'! = C) est calculée, mais son résultat sera toujours vrai.
En d'autres termes, le code examiné est équivalent à ceci:
for (c = getc (InFile); c==' ' || c == '\t'; c = getc (InFile))
Nous avons constaté que le code ne fonctionne pas comme prévu. Voyons maintenant quelles conséquences cela a.
Le programmeur qui a écrit l'appel
isspace dans le corps de la fonction
EatWhitespace s'attendait à ce qu'une fonction standard soit appelée. C'est pourquoi il a ajouté la condition selon laquelle le saut de ligne LF ('\ n') ne doit pas être considéré comme un espace.
Par conséquent, le programmeur prévoyait qu'en plus de l'espace et des onglets horizontaux, des caractères tels que le changement de page et l'onglet vertical seraient ignorés.
Il est à noter qu'il était également prévu de sauter le caractère de retour chariot CR (0x0d, '\ r'). Cela ne se produit pas et le cycle s'arrête lorsqu'il rencontre ce symbole. Cela entraînera des surprises désagréables si le séparateur de ligne dans le fichier est la séquence CR + LF utilisée sur certains systèmes non UNIX, tels que Microsoft Windows.
Pour ceux qui veulent en savoir plus sur les raisons historiques d'utiliser LF ou CR + LF comme séparateurs de ligne, voici l'article de Wikipedia "
Line feed ".
La fonction
EatWhitespace est
censée traiter les fichiers de la même manière, où LF et CR + LF sont utilisés comme séparateurs. Pour le cas de CR + LF, ce n'est pas le cas. En d'autres termes, si votre fichier provient du monde Windows, vous n'avez pas de chance :).
Ce n'est peut-être pas une grave erreur, d'autant plus que GNU Midnight Commander est courant sur les systèmes d'exploitation UNIX, où le caractère LF (0x0a, '\ n') est utilisé pour traduire une ligne. Cependant, en raison de telles bagatelles, divers problèmes gênants d'incompatibilité des données préparées dans les systèmes Linux et Windows se posent.
L'erreur décrite est intéressante en ce qu'elle est presque impossible à détecter avec une revue de code classique. Tous les développeurs de projets ne peuvent pas connaître les subtilités de la macro, et les oublier est très facile. C'est un bon exemple où l'analyse de code statique complète les revues de code et d'autres techniques de recherche d'erreurs.
Remplacer les fonctions standard est une mauvaise pratique. Soit dit en passant, récemment dans l'article «
Love Static Code Analysis », un cas similaire a été examiné avec la macro
#define sprintf std :: printf .
Une meilleure solution serait de donner à la macro un nom unique, par exemple,
is_space_or_tab . La confusion serait alors impossible.
La raison de la création de la macro est peut-être la lenteur de la fonction
isspace standard et le programmeur a créé une version plus rapide, suffisante pour résoudre toutes les tâches nécessaires. Mais encore, cette décision est erronée. Il serait plus
fiable de définir
isspace de manière à obtenir du code non compilé. Et pour implémenter les fonctionnalités nécessaires dans une macro avec un nom unique.
Merci de votre attention. Nous vous invitons à
télécharger et essayer l'analyseur PVS-Studio pour tester vos projets. De plus, nous vous rappelons que l'analyseur a récemment pris en charge le langage Java.
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Andrey Karpov.
Tu veux jouer un détective? Trouvez le bogue dans une fonction de Midnight Commander .