Code de ligne unique ou validation Nethermind à l'aide de PVS-Studio C # pour Linux

Image 1

Cet article est dédié au lancement du test bêta PVS-Studio C # pour Linux, ainsi que du plug-in pour Rider. Pour une si merveilleuse raison, en utilisant ces outils, nous avons vérifié le code source du produit Nethermind et dans cet article, nous examinerons des erreurs intéressantes et parfois drôles.

Nethermind est un client rapide pour .NET Core Ethereum pour Linux, Windows, MacOs. Il peut être utilisé dans des projets lors de la configuration de réseaux privés Ethereum ou dApps. Nethermind est une source ouverte située sur GitHub. Le projet a été fondé en 2017 et évolue constamment.

introduction


Aimez-vous le travail manuel? Par exemple, comme trouver des erreurs dans le code du programme. D'accord, il est plutôt fastidieux de lire et d'analyser le site que vous avez écrit ou l'ensemble du projet à la recherche d'un bug délicat. Eh bien, si le projet est petit, disons 5000 lignes, mais si sa taille a déjà dépassé cent mille ou un million de lignes? De plus, il a été écrit par différents programmeurs et parfois pas sous une forme très digeste. Que faire dans ce cas? Devez-vous vraiment dormir quelque part, quelque part sous-estimé et 100% de votre temps pour réaliser toutes ces lignes sans fin afin de comprendre où est cette méchante erreur? Je doute que vous aimeriez faire cela. Alors, que faire, peut-être existe-t-il des moyens modernes pour automatiser cela en quelque sorte?

figure 3

Et puis un outil comme un analyseur de code statique entre en jeu. L'analyseur statique est un outil pour détecter les défauts dans le code source des programmes. L'avantage de cet outil sur la vérification manuelle est le suivant:

  • ne passe presque pas votre temps à chercher l'emplacement de l'erreur (au moins c'est définitivement plus rapide que de chercher le copier-coller échoué avec vos yeux);
  • ne se fatigue pas, contrairement à une personne qui, après un certain temps de recherche, aura besoin de repos;
  • connaît de nombreux schémas d'erreur dont une personne peut même ne pas être au courant;
  • utilise des technologies telles que: analyse du flux de données, exécution symbolique, correspondance de modèles et autres;
  • vous permet d'analyser régulièrement et à tout moment;
  • etc.

Bien sûr, l'utilisation d'un analyseur de code statique ne remplace ni n'annule les révisions de code. Mais les revues de code deviennent plus productives et utiles. Vous pouvez vous concentrer sur la recherche d'erreurs de haut niveau, sur le transfert de connaissances, plutôt que de vous fatiguer à lire le code à la recherche de fautes de frappe.

Si vous êtes intéressé à en lire plus à ce sujet, je suggère l' article suivant , ainsi qu'un article sur les technologies utilisées dans PVS-Studio.

PVS-Studio C # pour Linux / macOS


Pour le moment, nous portons notre analyseur C # sur .NET Core, et nous développons également activement un plug-in pour IDE Rider.

Si vous êtes intéressé, vous pouvez vous inscrire à un test bêta en remplissant le formulaire sur cette page . Les instructions d'installation vous seront envoyées par e-mail (ne vous inquiétez pas, c'est très simple), ainsi qu'une licence pour utiliser l'analyseur.

Voici à quoi ressemble Rider avec le plugin PVS-Studio:

Image 4

Un peu d'indignation


Je tiens à dire qu'à certains endroits, il était très difficile de lire le code source de Nethermind, car des lignes de 300 à 500 caractères sont tout à fait normales. Oui, c'est tout le code sans mise en forme sur 1 ligne. Et ces lignes, par exemple, contiennent plusieurs opérateurs ternaires et opérateurs logiques, et il n'y a rien là-bas. En général, jouissance dès la dernière saison du jeu des trônes.

Je vais vous expliquer un peu pour comprendre la portée. J'ai un moniteur UltraWide d'une longueur d'environ 82 centimètres. Si vous ouvrez l'IDE dessus en plein écran, il contiendra environ 340 caractères. Autrement dit, les lignes dont je parle ne correspondent même pas. Si vous voulez voir à quoi ça ressemble, j'ai laissé des liens vers des fichiers sur GitHub:

Exemple 1

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    string authorString = (block.Author == null ? null : "sealed by " +
(KnownAddresses.GoerliValidators.ContainsKey(block.Author) ?
KnownAddresses.GoerliValidators[block.Author] : block.Author?.ToString())) ??
(block.Beneficiary == null ? string.Empty : "mined by " +
(KnownAddresses.KnownMiners.ContainsKey(block.Beneficiary) ?
KnownAddresses.KnownMiners[block.Beneficiary] : block.Beneficiary?.ToString()));
    if (_logger.IsInfo)
    {
        if (_logger.IsInfo) _logger.Info($"Discovered a new block
{string.Empty.PadLeft(9 - block.Number.ToString().Length, '
')}{block.ToString(Block.Format.HashNumberAndTx)} {authorString}, sent by
{syncPeer:s}");
    }
}

Exemple de lien de fichier

2

private void BuildTransitions()
{
    ...
    releaseSpec.IsEip1283Enabled = (_chainSpec.Parameters.Eip1283Transition ??
long.MaxValue) <= releaseStartBlock &&
((_chainSpec.Parameters.Eip1283DisableTransition ?? long.MaxValue) 
> releaseStartBlock || (_chainSpec.Parameters.Eip1283ReenableTransition ??
long.MaxValue) <= releaseStartBlock);           
    ...
}

Lien vers le fichier

public void 
Will_not_reject_block_with_bad_total_diff_but_will_reset_diff_to_null()
{
    ...
    _syncServer = new SyncServer(new StateDb(), new StateDb(), localBlockTree,
NullReceiptStorage.Instance, new BlockValidator(Always.Valid, new
HeaderValidator(localBlockTree, Always.Valid, MainnetSpecProvider.Instance,
LimboLogs.Instance), Always.Valid, MainnetSpecProvider.Instance, 
LimboLogs.Instance), Always.Valid, _peerPool, StaticSelector.Full, 
new SyncConfig(), LimboLogs.Instance);
    ...     
}

Lien vers le fichier

Imaginez maintenant qu'une erreur se produit dans une telle section, sera-t-il agréable de la chercher? Je suis sûr que ce n'est pas le cas, et tout le monde comprend qu'il est impossible d'écrire comme ça. Et au fait, il y a un endroit similaire avec une erreur dans ce projet.

Résultats de validation


Conditions qui n'aiment pas 0


Condition 1

public ReceiptsMessage Deserialize(byte[] bytes)
{
    if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
        return new ReceiptsMessage(null);
    ...
}

Avertissement PVS-Studio: V3106 L' index est peut-être hors limite. L'index '0' pointe au-delà de la limite des 'octets'. Nethermind.Network ReceiptsMessageSerializer.cs 50

Pour remarquer une erreur, considérez le cas où le nombre d'éléments dans le tableau est 0. Ensuite, la condition bytes.Length == 0 sera vraie et une exception indexOutOfRangeException de type 0 sera levée lors de l'accès à l'élément du tableau .

Ils voulaient quitter cette méthode immédiatement si le tableau est vide ou si l'élément 0 est égal à une certaine valeur, mais il semble qu'ils aient accidentellement mélangé "||" avec "&&". Je propose de résoudre ce problème comme suit:

public ReceiptsMessage Deserialize(byte[] bytes)
{
    if (bytes.Length == 0 || bytes[0] == Rlp.OfEmptySequence[0])
        return new ReceiptsMessage(null);
    ...
}

Condition 2

public void DiscoverAll()
{
    ...
    Type? GetStepType(Type[] typesInGroup)
    {
        Type? GetStepTypeRecursive(Type? contextType)
        {
            ...
        }
        ...
        return typesInGroup.Length == 0 ? typesInGroup[0] :
               GetStepTypeRecursive(_context.GetType());
    }
    ...
}

Avertissement PVS-Studio: V3106 L' index est peut-être hors limite. L'index «0» pointe au-delà de la limite «typesInGroup». Nethermind.Runner EthereumStepsManager.cs 70

Une situation similaire à celle décrite ci-dessus se produit. Si le nombre d'éléments dans typesInGroup est égal à 0, alors lors de l'accès à l'élément 0, une exception de type IndexOutOfRangeException sera levée .

Seulement dans ce cas, je ne comprends pas ce que voulait le développeur. Très probablement, au lieu de typesInGroup [0], vous devez écrire null .

Erreur ou optimisation inachevée?


private void DeleteBlocks(Keccak deletePointer)
{
   ...
   if (currentLevel.BlockInfos.Length == 1)
   {
      shouldRemoveLevel = true;
   }
   else
   {
      for (int i = 0; i < currentLevel.BlockInfos.Length; i++)
      {
         if (currentLevel.BlockInfos[0].BlockHash == currentHash) // <=
         {
            currentLevel.BlockInfos = currentLevel.BlockInfos
                                      .Where(bi => bi.BlockHash != currentHash)
                                      .ToArray();
            break;
         }
      }
   }
   ...
}

PVS-Studio Warning: V3102 Accès suspect à l'élément de l'objet 'currentLevel.BlockInfos' par un index constant à l'intérieur d'une boucle. Nethermind.Blockchain BlockTree.cs 895

À première vue, l'erreur est évidente - la boucle vise à itérer sur les éléments currentLevel.BlockInfos , mais lors de l'accès à currentLevel.BlockInfos [i], ils ont écrit currentLevel.BlockInfos [0] . Nous fixons 0 sur i et la mission est terminée. Mais ne vous précipitez pas, essayons de comprendre.

Maintenant, nous Longueur accès au BlockHash de l' élément nul. S'il est égal à currentHash , alors nous prenons de currentLevel.BlockInfos tous les éléments qui ne sont pas égauxcurrentHash , écrivez dessus et quittez la boucle. Il s'avère que le cycle est superflu.

Je pense que plus tôt il y avait un algorithme qu'ils ont décidé de changer / optimiser en utilisant linq , mais quelque chose s'est mal passé. Maintenant, dans le cas où la condition est fausse, nous obtenons des itérations sans signification.

Soit dit en passant, si le développeur qui a écrit cela utilisait le mode d' analyse incrémentielle , il s'est immédiatement rendu compte que quelque chose n'allait pas et aurait tout corrigé rapidement. Pour le moment, je réécrirais le code comme ceci:

private void DeleteBlocks(Keccak deletePointer)
{
    ...
    if (currentLevel.BlockInfos.Length == 1)
    {
        shouldRemoveLevel = true;
    }
    else
    {
        currentLevel.BlockInfos = currentLevel.BlockInfos
                                  .Where(bi => bi.BlockHash != currentHash)
                                  .ToArray();
    }
    ...
}

Déréférencer la référence nulle


Déréférencement 1

public void Sign(Transaction tx, int chainId)
{
    if (_logger.IsDebug)
        _logger?.Debug($"Signing transaction: {tx.Value} to {tx.To}");
    IBasicWallet.Sign(this, tx, chainId);
}

PVS-Studio Warning: V3095 L'objet '_logger' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 118, 118. Nethermind.Wallet DevKeyStoreWallet.cs 118

Erreur dans la mauvaise séquence. Il y a d'abord un appel à _logger.IsDebug et ce n'est qu'après que _logger est vérifié pour null . Par conséquent, dans le cas où _logger est nul , nous obtenons une exception de type NullReferenceException .

Déréférencement 2

private void BuildNodeInfo()
{
    _nodeInfo = new NodeInfo();
    _nodeInfo.Name = ClientVersion.Description;
    _nodeInfo.Enode = _enode.Info;                           // <=
    byte[] publicKeyBytes = _enode?.PublicKey?.Bytes;        // <=
    _nodeInfo.Id = (publicKeyBytes == null ? Keccak.Zero :
                   Keccak.Compute(publicKeyBytes)).ToString(false);
    _nodeInfo.Ip = _enode?.HostIp?.ToString();
    _nodeInfo.ListenAddress = $"{_enode.HostIp}:{_enode.Port}";
    _nodeInfo.Ports.Discovery = _networkConfig.DiscoveryPort;
    _nodeInfo.Ports.Listener = _networkConfig.P2PPort;
    UpdateEthProtocolInfo();
}

PVS-Studio Warning: V3095 L'objet '_enode' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 55, 56. Nethermind.JsonRpc AdminModule.cs 55 L'

erreur est complètement la même que celle décrite ci-dessus, mais cette fois, le coupable est _enode .

Je veux ajouter que si vous avez oublié de vérifier la valeur null, ne vous en souvenez que lorsque votre programme se bloque. L'analyseur vous le rappellera et tout ira bien.

Notre copier-coller préféré


Cas 1

public static bool Equals(ref UInt256 a, ref UInt256 b)
{
    return a.s0 == b.s0 && a.s1 == b.s1 && a.s2 == b.s2 && a.s2 == b.s2;
}

Avertissement PVS-Studio: V3001 Il existe des sous-expressions identiques 'a.s2 == b.s2' à gauche et à droite de l'opérateur '&&'. Nethermind.Dirichlet.Numerics UInt256.cs 1154

Ici, la même condition est vérifiée 2 fois:

a.s2 == b.s2

Puisque les paramètres a et b ont un champ s3 , je suppose que lors de la copie, nous avons simplement oublié de changer s2 en s3 .

Il s'avère que les paramètres seront égaux plus souvent que prévu par l'auteur du code. Dans le même temps, certains développeurs pensent qu'ils ne le font pas et commencent à chercher une erreur dans un endroit complètement différent, dépensant beaucoup d'énergie et de nerfs.

Soit dit en passant, les erreurs dans les fonctions de comparaison sont généralement un classique. Apparemment, les programmeurs, considérant ces fonctions comme simples, se rapportent à leur écriture de manière très désinvolte et distraite. Preuve . Sachant cela, maintenant soyez prudent :)!

Cas 2

public async Task<ApiResponse> 
PublishBlockAsync(SignedBeaconBlock signedBlock,
                  CancellationToken cancellationToken)
{
    bool acceptedLocally = false;
    ...
    if (acceptedLocally)
    {
        return new ApiResponse(StatusCode.Success);
    }
    else
    {
        return new ApiResponse(StatusCode.Success);
    }
    ...
}

PVS-Studio Warning: V3004 L' instruction 'then' est équivalente à l'instruction 'else'. Nethermind.BeaconNode BeaconNodeFacade.cs 177

Pour toute valeur de la acceptedLocally variable, la méthode renvoie la même. Il est difficile de dire s'il s'agit d'une erreur ou non. Supposons qu'un programmeur ait copié une ligne et oublié de changer StatusCode.Success à autre chose, alors c'est une vraie erreur. De plus, le StatusCode a une InternalError et InvalidRequest . Mais la refactorisation du code est probablement à blâmer et nous ne nous soucions pas de la valeur acceptée localement de toute façon, dans ce cas, la condition devient le lieu qui vous fait asseoir et penser si c'est une erreur ou non. Donc en tout cas, ce cas est extrêmement désagréable.

Cas 3

public void TearDown()
{
    ...
    foreach (var testResult in _results)
    {
         string message = $"{testResult.Order}. {testResult.Name} has " 
               + $"{(testResult.Passed ? "passed [+]" : "failed [-]")}";
         if (testResult.Passed)
         {
               TestContext.WriteLine(message);
         }
         else
         {
               TestContext.WriteLine(message);
         }
     }
}

PVS-Studio Warning: V3004 L' instruction 'then' est équivalente à l'instruction 'else'. Nethermind.Overseer.Test TestBuilder.cs 46

Et encore une fois, nous ne nous soucions pas de la vérification, car à la fin, nous obtenons la même chose. Et encore une fois, nous nous asseyons et souffrons, pensant, mais quel était le développeur qui voulait écrire ici. Une perte de temps qui aurait pu être évitée en utilisant l'analyse statique et en corrigeant immédiatement ce code ambigu.

Cas 4

public void Setup()
{
    if (_decoderBuffer.ReadableBytes > 0)
    {
        throw new Exception("decoder buffer");
    }

    if (_decoderBuffer.ReadableBytes > 0)
    {
        throw new Exception("decoder buffer");
    }
    ...
}

PVS-Studio Warning: V3021 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la méthode return. Cela signifie que le second « si » déclaration est InFlowBenchmarks.cs insensée Nethermind.Network.Benchmark 55 Il

suffit de frapper Ctrl + V à nouveau . Nous supprimons l'excédent de chèque et tout va bien. Je suis sûr que si une autre condition importante était ici, alors tout le monde écrirait dans un si par l'opérateur logique I.

Cas 5

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    if (_logger.IsInfo)
    {
        if (_logger.IsInfo)
        {
            ...
        }
    }
}

Avertissement PVS-Studio: vérification récurrente V3030 . La condition '_logger.IsInfo' a déjà été vérifiée à la ligne 242. Nethermind.Synchronization SyncServer.cs 244

De la même manière que dans le quatrième cas, une vérification supplémentaire est effectuée. Cependant, la différence est que _logger a non seulement une propriété, mais aussi, par exemple, ' bool IsError {get; } '. Par conséquent, le code devrait probablement ressembler à ceci:

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    if (_logger.IsInfo)
    {
        if (!_logger.IsError) // <=
        {
            ...
        }
    }
}

Eh bien, ou tout le défaut du refactoring de code et une seule vérification n'est plus nécessaire.

Cas 6

if (missingParamsCount != 0)
{
    bool incorrectParametersCount = missingParamsCount != 0; // <=
    if (missingParamsCount > 0)
    {
        ...
    }
    ...
}

Avertissement PVS-Studio: l' expression V3022 'missingParamsCount! = 0' est toujours vraie. Nethermind.JsonRpc JsonRpcService.cs 127

Vérifiez la condition (missingParamsCount! = 0) et si elle est vraie, nous la calculons à nouveau et affectons le résultat à une variable. Convenez que c'est une façon assez originale d'écrire vrai.

Chèque déroutant


public async Task<long> 
DownloadHeaders(PeerInfo bestPeer, 
                BlocksRequest blocksRequest, 
                CancellationToken cancellation)
{
  ...
  for (int i = 1; i < headers.Length; i++)
  {
    ...
    BlockHeader currentHeader = headers[i];
    ...
    bool isValid = i > 1 ? 
        _blockValidator.ValidateHeader(currentHeader, headers[i - 1], false):
        _blockValidator.ValidateHeader(currentHeader, false);
    ...
    if (HandleAddResult(bestPeer, 
                        currentHeader, 
                        i == 0,                              // <=
                        _blockTree.Insert(currentHeader))) 
    {
       headersSynced++;
    }

    ...
  }
  ...
}

Avertissement PVS-Studio: l' expression V3022 'i == 0' est toujours fausse. Nethermind.Synchronization BlockDownloader.cs 192

Commençons dans l'ordre. Lors de l'initialisation, la variable i se voit attribuer la valeur 1. Ensuite, la variable est uniquement incrémentée, par conséquent, la valeur sera toujours transmise à la fonction false .

Voyons maintenant HandleAddResult :

private bool HandleAddResult(PeerInfo peerInfo, 
                             BlockHeader block,
                             bool isFirstInBatch, 
                             AddBlockResult addResult)
{
    ...
    if (isFirstInBatch)
    {
        ...
    }
    else
    {
        ...
    }
    ...
}

Ici, nous sommes intéressés par isFirstInBatch . A en juger par le nom de ce paramètre, il est responsable de savoir si quelque chose est le premier dans le parti. Hm, d'abord. Regardons à nouveau ci-dessus et voyons qu'il y a 2 appels en utilisant i :

BlockHeader currentHeader = headers[i];
_blockValidator.ValidateHeader(currentHeader, headers[i - 1], false)

N'oubliez pas que le compte à rebours dans ce cas passe de 1. Il s'avère que nous avons 2 options: soit le «premier» signifie un élément sous l'index 1, soit sous l'index 0. Mais dans tous les cas, ce sera égal à i .

Il s'avère que l'appel de fonction devrait ressembler à ceci:

HandleAddResult(bestPeer, currentHeader, 
                i == 1, _blockTree.Insert(currentHeader))

Ou comme ça:

HandleAddResult(bestPeer, currentHeader, 
                i - 1 == 0, _blockTree.Insert(currentHeader))

Et encore une fois, si le développeur utilisait constamment un analyseur statique, il écrirait ce code et verrait un avertissement, le corrigerait rapidement et profiterait de la vie.

Une priorité ??


Situation 1

public int MemorySize
{
  get
  {
    int unaligned = (Keccak == null ? MemorySizes.RefSize : 
        MemorySizes.RefSize + Keccak.MemorySize) 
        + (MemorySizes.RefSize + FullRlp?.Length 
                                 ?? MemorySizes.ArrayOverhead)   // <=
        + (MemorySizes.RefSize + _rlpStream?.MemorySize 
                                 ?? MemorySizes.RefSize)         // <=
        + MemorySizes.RefSize + (MemorySizes.ArrayOverhead + _data?.Length 
        * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead) 
        + MemorySizes.SmallObjectOverhead + (Key?.MemorySize ?? 0);
    return MemorySizes.Align(unaligned);
  }
}

Avertissements de PVS-Studio:

  • V3123 Peut-être le '??' L'opérateur fonctionne d'une manière différente de celle attendue. Sa priorité est inférieure à la priorité des autres opérateurs dans sa partie gauche. Nethermind.Trie TrieNode.cs 43
  • V3123 Peut-être le '??' L'opérateur fonctionne d'une manière différente de celle attendue. Sa priorité est inférieure à la priorité des autres opérateurs dans sa partie gauche. Nethermind.Trie TrieNode.cs 44

L'analyseur vous conseille de vérifier comment nous utilisons les opérateurs "??", et pour comprendre quel est le problème, je propose de considérer la situation suivante. Nous regardons cette ligne ici:

(MemorySizes.RefSize + FullRlp?.Length ?? MemorySizes.ArrayOverhead)

MemorySizes.RefSize et MemorySizes.ArrayOverhead sont des constantes:

public static class MemorySizes
{
    ...
    public const int RefSize = 8;
    public const int ArrayOverhead = 20;
    ...
}

Par conséquent, pour plus de clarté, je propose de réécrire la chaîne, en remplaçant leurs valeurs:

(8 + FullRlp?.Length ?? 20)

Supposons maintenant que FullRlp soit nul. Alors (8 + null) sera nul . Ensuite, nous obtenons l'expression ( null ?? 20 ), qui retournera 20.

Il s'avère que, à condition que FullRlp soit null , la valeur de MemorySizes.ArrayOverhead sera toujours renvoyée, indépendamment de ce qui est stocké dans MemorySizes.RefSize. Le fragment sur la ligne ci-dessous est similaire.

Mais la question est, le développeur voulait-il ce comportement? Regardons la ligne suivante:

MemorySizes.RefSize + (MemorySizes.ArrayOverhead 
    + _data?.Length * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead) 

Ici, comme dans les sections ci-dessus, MemorySizes.RefSize est ajouté à l'expression, mais

notez qu'après le premier opérateur «+», il y a un crochet. Il s'avère que nous devons ajouter une expression à MemorySizes.RefSize , et si elle est nulle , nous en ajouterons une autre. Le code devrait donc ressembler à ceci:

public int MemorySize
{
  get
  {
    int unaligned = (Keccak == null ? MemorySizes.RefSize : 
       MemorySizes.RefSize + Keccak.MemorySize) 
       + (MemorySizes.RefSize + (FullRlp?.Length 
                                 ?? MemorySizes.ArrayOverhead))    // <=
       + (MemorySizes.RefSize + (_rlpStream?.MemorySize 
                                 ?? MemorySizes.RefSize))          // <=
       + MemorySizes.RefSize + (MemorySizes.ArrayOverhead + _data?.Length 
       * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead) 
       + MemorySizes.SmallObjectOverhead + (Key?.MemorySize ?? 0);
    return MemorySizes.Align(unaligned);
  }
}

Encore une fois, ce n'est qu'une hypothèse, cependant, si le développeur souhaite un comportement différent, vous devez l'indiquer explicitement:

((MemorySizes.RefSize + FullRlp?.Length) ?? MemorySizes.ArrayOverhead)

Et puis, celui qui lit ce code n'aurait pas à se plonger longtemps dans ce qui se passe ici et ce que le programmeur voulait.

Situation 2

private async Task<JsonRpcResponse> 
ExecuteAsync(JsonRpcRequest request, 
             string methodName,
             (MethodInfo Info, bool ReadOnly) method)
{
    var expectedParameters = method.Info.GetParameters();
    var providedParameters = request.Params;
    ...
    int missingParamsCount = expectedParameters.Length 
            - (providedParameters?.Length ?? 0) 
            + providedParameters?.Count(string.IsNullOrWhiteSpace) ?? 0; // <=
    if (missingParamsCount != 0)
    {
        ...
    }
    ...
}

Avertissement PVS-Studio: V3123 Peut-être le '??' L'opérateur fonctionne d'une manière différente de celle attendue. Sa priorité est inférieure à la priorité des autres opérateurs dans sa partie gauche. Nethermind.JsonRpc JsonRpcService.cs 123

Et encore une fois, la priorité de l'opération est "??", par conséquent, comme la dernière fois, nous considérons la situation. Nous regardons cette ligne ici:

expectedParameters.Length 
            - (providedParameters?.Length ?? 0) 
            + providedParameters?.Count(string.IsNullOrWhiteSpace) ?? 0;

Supposons que providedParameters est null , puis pour plus de clarté, remplacez immédiatement tout ce qui est lié à providedParameters par null, et remplacez une valeur aléatoire au lieu de expectParameters.Length :

100 - (null ?? 0) + null ?? 0;

Maintenant, on remarque immédiatement qu'il y a deux contrôles similaires, seulement dans un cas il y a des crochets, et dans l'autre là. Exécutons cet exemple. D'abord, nous obtenons que ( null ?? 0 ) renverra 0, puis soustrayez 0 de 100 et obtenez 100:

100 + null ?? 0;

Maintenant, au lieu d'exécuter d'abord " null ?? 0 " et d'obtenir finalement ( 100 + 0 ), nous obtenons complètement différent.

D'abord, il s'exécutera ( 100 + null ) et nous obtiendrons null . Ensuite, il vérifie ( null ?? 0 ), ce qui conduit au fait que la valeur de la variable missingParamsCount sera 0.

Comme la condition suivante est que le missingParamsCount est égal à zéro, nous pouvons supposer que c'est le comportement recherché par le développeur. Et je dirai, pourquoi ne pas mettre des parenthèses et exprimer mes pensées de manière explicite? En général, il est possible que cette vérification soit due à une mauvaise compréhension de la raison pour laquelle 0 est parfois retourné, et ce n'est rien de plus qu'une béquille.

Et encore une fois, nous perdons du temps, bien que nous ne l'ayons peut-être pas fait; utilisez le mode d' analyse incrémentielle lors de l'écriture de code .

Conclusion


En conclusion, j'espère avoir pu vous transmettre que l'analyseur statique est votre ami, et non un mauvais surveillant qui n'attend que vous pour faire une erreur.

Il convient également de noter qu'en utilisant l'analyseur une fois ou en l'utilisant rarement, vous trouverez bien sûr des erreurs et certaines d'entre elles seront même rapidement corrigées, mais il y en aura aussi dont vous devrez vous casser la tête. Par conséquent, vous devez constamment utiliser un analyseur statique. Ensuite, vous trouverez de nombreuses autres erreurs et les corrigerez au moment où le code du programme est écrit et vous comprendrez exactement ce que vous essayez de faire.

La simple vérité est que tout le monde fait des erreurs et c'est normal. Nous apprenons tous des erreurs, mais seulement de celles qui ont pu le remarquer et le comprendre. Par conséquent, utilisez des outils modernes pour rechercher ces mêmes erreurs, par exemple - PVS-Studio. Merci pour l'attention.



Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Nikolay Mironov. Code sur une seule ligne ou vérification de Nethermind à l'aide de PVS-Studio C # pour Linux .

All Articles