Analyse du code ajouté Visual Studio ( La plupart des problèmes génèrent car l'ancien code fait des affirmations au début de la méthode ou de la fonction. P> Je pense que c'est la définition d'assertion utilisée dans le code (à partir de code d'exemple: / Analyse Code>) pour C / C ++ afin d'aider à identifier le mauvais code. C'est une fonctionnalité assez belle, mais lorsque vous traitez avec un ancien projet, vous pouvez être submergé par le nombre d'avertissements. AFX.H code>) p> ASSERT(pBytes != NULL);
*pBytes = 0; // <- warning C6011: Dereferencing NULL pointer 'pBytes'
6 Réponses :
Vous semblez supposer que assert (PTR) signifie en quelque sorte que pTR n'est pas null ensuite. Ce n'est pas vrai et l'analyseur de code ne fait pas cette hypothèse.
Ceci est vrai, mais cela ne répond pas à la question.
Prefast vous dit que vous avez un défaut de votre code; Ne l'ignorez pas. En fait, vous en avez un, mais vous n'avez que Scittered autour de la reconnaître. Le problème est la suivante: juste parce que / rant Il y a deux façons de résoudre ce problème: La bonne façon, et un hack. Le bon moyen consiste à gérer les pointeurs nuls au moment de l'exécution: Cela fera silencer le préfast. Le hack doit utiliser une annotation. Par exemple: édition: Voici un lien décrivant les annotations de Prefast. Un point de départ, de toute façon. pbytes n'a jamais été nul dans le développement et les tests ne signifie pas que ce ne sera pas en production. Vous ne gérez pas cette éventualité. Prefast le sait et tente de vous avertir que les environnements de production sont hostiles et laisseront votre code une masse de fumer, mutilée d'octets sans valeur.
J'avais peur que c'était la seule solution car elle nécessite une grande quantité de code à ajouter.
Une autre option pourrait être de redéfinir l'affirmation pour inclure l'annotation. Mieux vaut toujours être de redéfinir l'affirmation afin qu'elle fonctionne dans la libération et gère les pointeurs nuls de manière significative.
Euh, vous supposez aveuglément que pbytes peut en fait être null . Ce n'est pas nécessairement une hypothèse valide. Prefast vient de jouer en sécurité: cela donne souvent de faux positifs. C'est pourquoi cela n'est pas activé par défaut. Il vous indique ce qu'il trouve suspect et à vous de vérifier que c'est correct, et si oui, résolvez le problème. Mais dire que vous avez réellement besoin de gérer le boîtier NULL simplement parce que PREFAST dit que les ordures pures. Manipulez-le s'il peut réellement se produire logiquement.
Les affirmations sont conçues pour attraper des erreurs logiques: des choses qui ne peuvent jamais se produire logiquement. Et ceux-ci sont par définition impossible à récupérer de. Vous essayiez de récupérer d'un problème que vous n'avez pas identifié. Comment savez-vous quoi faire pour récupérer alors? En supposant que l'affirmation dans ce cas est correctement utilisée (pas comme une manipulation des erreurs, mais de se protéger des erreurs logiques pendant le développement) et de supposer que la valeur ne peut pas logiquement être null , il n'y a absolument aucune raison de gérer le cas où il est null .
@jalf: Utiliser un pointeur sans vérifier qu'il est téméraire. C'est la programmation 101.
@jalf: J'ai remarqué que vous n'aviez pas d / V l'autre réponse qui n'a affirmé que ce que j'ai affirmé - que le pointeur pourrait en fait être nul - et n'a même pas fourni les moyens de faire taire le préférentiel. My Rant a-t-il touché un nerf?
@John: Donc, ce code est téméraire? int i; int * p = & i; * p = 42; soin de m'expliquer comment cela peut se tromper?
@John: Oui, cela a touché un nerf en fait. Je pense que ne pas comprendre votre propre code est une mauvaise pratique de programmation. Vérification aveuglée contre chaque condition d'erreur, même celles qui ne peuvent pas se produire, ne vous aide pas à écrire plus de logiciels plus robustes. Vous réalisez des logiciels robustes en manipulant les erreurs que peut survenir surviennent, sans la vaste complexité supplémentaire de gérer tous les cas d'erreur que ne peut pas . Plus de complexité ne donne pas de code plus sûr.
@jalf: votre exemple posté ne échoue pas. Malheureusement, le monde réel n'est pas si simple. Dans le monde réel, les choses changent sans votre contrôle ou votre connaissance tout le temps. D'autres personnes appellent vos fonctions, parfois incorrectement. D'autres modules interagissent avec votre module de différentes manières. Changement de protocoles de socket. Etc. Alors dis-moi - lundi matin lorsque le serveur OPRA que vous êtes connecté pour commencer à cracher Gibberish, si votre code s'écrase? Ou signalez une erreur et essayez de le gérer, peut-être en étant gracieusement?
@John: Exactement, mon exemple posté ne échoue pas. Leur «utilisation d'un pointeur sans le vérifier» n'est pas toujours téméraire. Si vous pouvez prouver qu'il est sûr, il n'est pas nécessaire de gérer le cas où il échoue. C'est mon point. Évidemment, si vous pouvez pas prouver qu'il est toujours en sécurité, alors vous avez raison, il doit être manipulé. Mon point est simplement que si c'est sûr, vous gaspillez votre temps de traiter une question inexistante. (Vous pourriez aussi bien essayer de vérifier que 2 + 2 = 4, ou que lorsque je appelle la fonction foo () , bar () ne s'appelle pas.)
@John est correct - vous Doit vérifier la valeur du pointeur entrant, au moins pour NULL, car l'affirmation () disparaît dans une version de version. C'est pas un préféffrant faux positif.
@ John Dibling - Une méthode privée dans une version de libération ne doit pas nécessiter de vérifier chacun de ses paramètres à chaque fois. C'est un gaspillage de ressources. La méthode doit être appelée correctement en premier lieu. À chaque fois. Si vous ne pouvez pas vérifier en inspectant qu'une méthode privée est utilisée correctement, vous ne codez pas correctement. Si vous ne pouvez jamais supposer que les paramètres à une méthode privée seront corrects, alors quoi sur Terre est-elle affirmée? Dans ce cas, nous devrions simplement faire de la libération la même que la construction de débogage et vérifier les mêmes valeurs contre NULL encore et encore tout le temps.
@Jeffrey: Depuis quand parle-t-on de méthodes privées?
@John Dibling - Depuis que la OP a mis une affirmation dans le code. Si vous ne contrôlez pas le code d'appel, vous devez vérifier les paramètres de la version de version.
@Jeffrey: Préférez-vous que les affirmations n'appartiennent pas à des méthodes appelées par code que nous ne contrôlons pas?
@Michael Howard-MSFT: Comment pouvez-vous dire? Affirmer n'a rien à voir avec ça. Certains codes quelques lignes * ci-dessus auraient pu s'assurer que le pointeur ne sera jamais nul, sans que le pratiquait de la capturer. Je (ou quelqu'un d'autre) n'a jamais prétendu que le affirmer a fait quelque chose pour l'empêcher.
@John: On dirait que ça me va. Si vous ne pouvez pas contrôler comment et à partir de l'endroit où la fonction est appelée, vous devez évidemment vérifier les paramètres. Quelqu'un pourrait l'appeler avec de mauvais paramètres. Ce que @jeffrey et moi disent, c'est que si vous contrôlez chaque point d'entrée dans le code, vous pouvez vous assurer que les paramètres seront toujours valides et alors une affirmation est Parfaitement approprié: nous devons simplement nous assurer que notre logique est sonore, rien de plus.
Il est irréaliste de s'attendre à avoir des avertissements zéro avec / analyser.
Il a souligné une situation où vous avez une déréférence un pointeur que il ne peut pas vérifier est toujours valable. Autant que Prefast puisse dire, rien ne garantit que cela ne sera jamais nul.
Mais cela ne signifie pas que peut être null. Juste que l'analyse requise pour prouver que c'est sûr est trop complexe pour PREFAST.
Vous pourrez peut-être utiliser l'extension spécifique à Microsoft Si vous utilisez vos affirmations correctement (pour attraper une erreur logique lors de la programmation, la protection contre les situations ne peut pas être se produire, je ne vois aucun problème avec votre code ou en laissant l'avertissement. Ajout du code à gérer. Un problème que peut ne jamais se produire n'est jamais inutile. Vous ajoutez plus de code et plus de complexité sans raison (si cela ne peut jamais se produire, vous n'avez aucun moyen de vous en remettre, car vous n'avez absolument pas Aucun indice dans quel état le programme sera dans. Tout ce que vous savez, c'est qu'il est entré dans un chemin de code que vous pensiez impossible.
Toutefois, si vous utilisez votre affirmation comme traitement des erreurs réelles (la valeur peut être null dans des cas exceptionnels, vous vous attendez à ce que cela ne se produise pas), il s'agit d'un défaut de votre code. . Ensuite, une bonne gestion des erreurs (exceptions, typiquement) est nécessaire.
N'utilisez jamais d'affirmations pour des problèmes qui sont possibles . Utilisez-les pour vérifier que le impossible ne se produit pas. Et quand / Analyser vous donne des avertissements, regardez-les. S'il s'agit d'un faux positif, ignorez-le (ne le supprimez pas, car il s'agit d'un faux positif aujourd'hui, le code que vous vérifiez demain peut la transformer en un vrai problème). / analyser n'est pas garanti de donner des avertissements pertinents et corrects.
Cela peut manquer beaucoup de problèmes, et cela donne également un certain nombre de faux positifs (les choses qu'il identifie comme des avertissements, mais qui sont parfaitement sûres et ne se produiront jamais)
__ suppose pour dire au compilateur qu'il ne faut pas produire cet avertissement, mais une meilleure solution consiste à laisser l'avertissement. Chaque fois que vous compilez avec / analysez (qui ne nécessitez pas à chaque fois que vous compilez), vous devriez vérifier que les avertissements qu'il propose sont toujours faux positifs.
Je n'essaie pas de commencer une guerre avec toi, mais je crois que c'est très mauvais conseil: "Une meilleure solution consiste à laisser l'avertissement. Chaque fois que vous compilez avec / analysez [...], vous devriez vérifier que le AVERTISSEMENTS Il est proposé des faux positifs. " Mon raisonnement dans le prochain commentaire ...
Mon opinion doit être nécessaire pour pouvoir compiler avec des erreurs zéro et des avertissements zéro sur les niveaux d'alerte les plus élevés (y compris / analyser) avant qu'un enregistrement ne soit autorisé. De nombreuses raisons, mais une très simple est que si des avertissements sont autorisés à rester alors que vous risquez de vous retrouver avec des centaines ou des milliers d'avertissements. Il est déraisonnable de s'attendre à ce que chacun soit vérifié pour la validité chaque fois qu'une construction de production est faite. Cela ne se produira tout simplement pas, vaincre le but des avertissements en premier lieu.
@John: C'est pourquoi vous ne compilez généralement pas avec / analysez. Mais bien sûr, si vous préférez les avertissements zéro même avec / analyser, n'hésitez pas à faire taire des avertissements une fois qu'ils ont été confirmés pour être faux positifs. Mon point principal est simplement que vous n'avez pas besoin de gérer des problèmes qui ne peuvent jamais se produire. Que vous soyez ou non si vous faites taire l'avertissement pour cela est à vous de décider. Je préférerais le garder, et il suffit de courir / analyser uniquement à l'occasion, car Prefast n'est pas supposé être précis ou donner zéro faux positifs. Il est censé être paranoïaque, et donc je veux une fois de temps en temps vérifier sa sortie.
N'oubliez pas que Assert () disparaît dans une version de vente au détail de sorte que l'avertissement C6011 est absolument correct dans votre code ci-dessus: vous devez vérifier que les Pbytes sont non nuls et que font l'affirmation (). L'affirmation () jette simplement votre application dans un débogueur si cette condition est remplie dans un bogue de débogage.
Je travaille beaucoup sur / Analyser et Prefast, donc si vous avez d'autres questions, n'hésitez pas à me le faire savoir.
Vous supposez trop du code de l'OP. Oui, l'assert disparaît dans une version de libération, nous ne pouvons donc pas compter sur l'affirmation seule pour vérifier que le pointeur est non nul. Mais probablement, si l'assert est utilisé, c'est parce que le programmateur sait par d'autres moyens que le pointeur ne peut jamais être nul. Si tel est le cas, si l'affirmation est utilisée correctement, cela est simplement un faux positif. S'il s'appuie effectivement sur l'affirmation pour attraper des pointeurs nuls que réellement se produisent comme vous le soupçonnez, il abuse alors de l'affirmation et devrait la remplacer par une manipulation d'erreur appropriée.
Qu'est-ce qui est un bon moyen d'inclure les annotations de préfecture mais permettent toujours de construire le code avec GCC?
Mon co-auteur David LeBlanc me dirait que ce code est brisé de toute façon, en supposant que vous utilisiez C ++, vous devez utiliser une référence plutôt qu'un pointeur, et une référence ne peut pas être null :)
Tout d'abord, votre relevé d'affirmation doit garantir de lancer ou de mettre fin à l'application. Après une certaine expérimentation, j'ai trouvé dans ce cas / Analyser ignore toute mise en œuvre dans les fonctions de modèle, des fonctions en ligne ou des fonctions normales. Vous devez plutôt utiliser des macros et le do {} tandis que (0) astuce, avec une suppression inline de
si vous regardez la définition de Atlensure () Microsoft utilise __analyse_assume () Dans leur macro, ils ont également plusieurs paragraphes de très bonne documentation sur les raisons et comment elles sont Migration de ATL pour utiliser cette macro.
Comme exemple, j'ai modifié les macros CPPUnit_assert de la même manière pour nettoyer des milliers d'avertissements dans nos tests unitaires.
Pourriez-vous montrer certains des messages d'avertissement?
Pouvez-vous également montrer votre implémentation de
assert ?/ analyser ne se plaint pas suraffirme sur ma machine.J'ai mis à jour la description, BTW / Analyze ne se plaignai pas d'affirmer, mais il est conscient que l'affirmation peut être ignorée et comme effet secondaire, cela déclenchera l'avertissement lorsque la variable est utilisée.