Dans cet article, nous vous invitons à essayer de trouver un bogue dans une fonction très simple du projet GNU Midnight Commander. Pourquoi? Sans raison particulière. Juste pour le plaisir. Eh bien, c'est un mensonge. Nous voulions en fait vous montrer un autre bug qu'un critique humain a du mal à trouver et que l'analyseur de code statique PVS-Studio peut détecter sans effort.
Un utilisateur nous a envoyé un e-mail l'autre jour, lui demandant pourquoi il recevait un avertissement sur la fonction
EatWhitespace (voir le code ci-dessous). Cette question n'est pas aussi triviale qu'elle puisse paraître. Essayez de découvrir par 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,
EatWhitespace est une fonction minuscule; son corps est encore plus petit que le commentaire :). Maintenant, vérifions quelques détails.
Voici la description de la fonction
getc :
int getc ( FILE * stream );
Renvoie le caractère actuellement pointé par l'indicateur de position de fichier interne du flux spécifié. L'indicateur de position de fichier interne est ensuite avancé au caractère suivant. Si le flux se trouve à la fin du fichier lors de son appel, la fonction renvoie
EOF et définit l'indicateur de fin de fichier pour le flux. Si une erreur de lecture se produit, la fonction renvoie EOF et définit l'indicateur d'erreur pour le flux (ferror).
Et voici la description de la fonction
isspace :
int isspace( int ch );
Vérifie si le caractère donné est un caractère d'espacement tel que classé par les paramètres régionaux C actuellement installés. Dans les paramètres régionaux par défaut, les caractères d'espacement sont les suivants:
- espace (0x20, '');
- flux de formulaire (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 non nulle si le caractère est un espace blanc; zéro sinon.
La fonction
EatWhitespace devrait ignorer tous les caractères d'
espacement sauf le saut de ligne '\ n'. La fonction arrêtera également la lecture du fichier lorsqu'elle rencontrera Fin de fichier (EOF).
Maintenant que vous savez tout cela, essayez de trouver le bug!
Les deux licornes ci-dessous vous assureront de ne pas jeter un coup d'œil accidentel au commentaire.
Figure 1. Temps de recherche de bogues. Les licornes attendent.Toujours pas de chance?
Eh bien, vous voyez, c'est parce que nous vous avons menti à propos de
isspace . Bwa-ha-ha! Ce n'est pas du tout une fonction standard - c'est une macro personnalisée. Ouais, nous sommes méchants et nous vous avons confondu.
Figure 2. Licorne déroutant les lecteurs sur isspace .Ce n'est pas nous ou notre licorne à blâmer, bien sûr. La faute de toute confusion réside dans les auteurs du projet GNU Midnight Commander, qui ont fait leur propre implémentation de
isspace dans le fichier charset.h:
#ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t')
Avec cette macro, les auteurs ont confondu d'autres développeurs. Le code a été écrit en supposant que
isspace est une fonction standard, qui considère le retour chariot (0x0d, '\ r') comme un espace blanc.
La macro personnalisée, à son tour, traite uniquement les espaces et les tabulations comme des espaces. Remplaçons cette macro et voyons ce qui se passe.
for (c = getc (InFile); ((c)==' ' || (c) == '\t') && ('\n' != c); c = getc (InFile))
La sous-expression ('\ n'! = C) n'est pas nécessaire (redondante) car elle sera toujours évaluée à true. C'est ce que PVS-Studio vous avertit en émettant l'avertissement:
V560 Une partie de l'expression conditionnelle est toujours vraie: ('\ n'! = C). params.c 136.
Pour être clair, examinons 3 résultats possibles:
- Fin de fichier atteinte. EOF n'est pas un espace ou un caractère de tabulation. La sous-expression ('\ n'! = C) n'est pas évaluée en raison d'une évaluation de court-circuit . La boucle se termine.
- La fonction a lu un caractère qui n'est ni un espace ni un caractère de tabulation. La sous-expression ('\ n'! = C) n'est pas évaluée en raison d'une évaluation de court-circuit. La boucle se termine.
- La fonction a lu un espace ou un caractère de tabulation horizontale. La sous-expression ('\ n'! = C) est évaluée, mais son résultat est toujours vrai.
En d'autres termes, le code ci-dessus est équivalent à ce qui suit:
for (c = getc (InFile); c==' ' || c == '\t'; c = getc (InFile))
Nous avons constaté que cela ne fonctionne pas de la manière souhaitée. Voyons maintenant quelles sont les implications.
Un développeur, qui a écrit l'appel de
isspace dans le corps de la fonction
EatWhitespace , s'attendait à ce que la fonction standard soit appelée. C'est pourquoi ils ont ajouté la condition empêchant le caractère LF ('\ n') d'être traité comme un espace blanc.
Cela signifie que, outre l'espace et les tabulations horizontales, ils prévoyaient également d'ignorer les sauts de page et les tabulations verticales.
Ce qui est plus remarquable, c'est qu'ils voulaient que le caractère de retour chariot (0x0d, '\ r') soit également ignoré. Cela ne se produit cependant pas - la boucle se termine lorsque vous rencontrez ce personnage. Le programme finira par se comporter de manière inattendue si les sauts de ligne sont représentés par la séquence CR + LF, qui est le type utilisé dans certains systèmes non UNIX tels que Microsoft Windows.
Pour plus de détails sur les raisons historiques de l'utilisation de LF ou CR + LF comme caractères de nouvelle ligne, voir la page Wikipedia "
Nouvelle ligne ".
La fonction
EatWhitespace était censée traiter les fichiers de la même manière, qu'ils utilisent LF ou CR + LF comme caractères de nouvelle ligne. Mais il échoue dans le cas de CR + LF. En d'autres termes, si votre fichier provient du monde Windows, vous avez des problèmes :).
Bien que cela ne soit pas un bug sérieux, surtout si l'on considère que GNU Midnight Commander est utilisé dans des systèmes d'exploitation de type UNIX, où LF (0x0a, '\ n') est utilisé comme caractère de nouvelle ligne, des bagatelles comme celle-ci ont toujours tendance à être ennuyeuses problèmes de compatibilité des données préparées sous Linux et Windows.
Ce qui rend ce bogue intéressant, c'est que vous êtes presque sûr de l'oublier lors de la révision de code standard. Les détails de l'implémentation de la macro sont faciles à oublier et certains auteurs de projets peuvent ne pas les connaître du tout. C'est un exemple très vivant de la façon dont l'analyse de code statique contribue à la révision du code et à d'autres techniques de détection de bogues.
Remplacer les fonctions standard est une mauvaise pratique. À propos, nous avons discuté d'un cas similaire de la macro
#define sprintf std :: printf dans le récent article "
Appreciate Static Code Analysis ".
Une meilleure solution aurait été de donner à la macro un nom unique, par exemple,
is_space_or_tab . Cela aurait permis d'éviter toute confusion.
Peut-être que la fonction standard
isspace était trop lente et le programmeur a créé une version plus rapide, suffisante pour leurs besoins. Mais ils n'auraient toujours pas dû le faire de cette façon. Une solution plus sûre serait de définir
isspace pour obtenir du code non compilable, tandis que la fonctionnalité souhaitée pourrait être implémentée sous forme de macro avec un nom unique.
Merci d'avoir lu. N'hésitez pas à
télécharger PVS-Studio et à l'essayer avec vos projets. Pour rappel, nous prenons désormais également en charge Java.