Einzeiliger Code oder Nethermind-Validierung mit PVS-Studio C # für Linux

Bild 1

Dieser Artikel ist dem Start des PVS-Studio C # Beta-Tests für Linux sowie des Plug-Ins für Rider gewidmet. Aus diesem wunderbaren Grund haben wir mit diesen Tools den Quellcode des Nethermind-Produkts überprüft und in diesem Artikel werden wir interessante und manchmal lustige Fehler untersuchen.

Nethermind ist ein schneller Client für .NET Core Ethereum für Linux, Windows und MacOs. Es kann in Projekten verwendet werden, wenn private Netzwerke von Ethereum oder dApps eingerichtet werden. Nethermind ist Open Source auf GitHub. Das Projekt wurde 2017 gegründet und entwickelt sich ständig weiter.

Einführung


Magst du Handarbeit? Zum Beispiel das Auffinden von Fehlern im Programmcode. Stimmen Sie zu, es ist ziemlich mühsam, die von Ihnen geschriebene Site oder das gesamte Projekt zu lesen und zu analysieren, um nach einem kniffligen Fehler zu suchen. Nun, wenn das Projekt klein ist, sagen wir 5.000 Zeilen, aber wenn seine Größe bereits hunderttausend oder eine Million Zeilen überschritten hat? Darüber hinaus wurde es von verschiedenen Programmierern geschrieben und manchmal nicht in einer sehr leicht verdaulichen Form. Was ist in diesem Fall zu tun? Ist es wirklich notwendig, irgendwo zu schlafen, irgendwo zu unterschätzen und 100% Ihrer Zeit zu verbringen, um all diese endlosen Linien zu erkennen, um zu verstehen, wo dieser böse Fehler liegt? Ich bezweifle, dass Sie dies gerne tun würden. Was tun, vielleicht gibt es moderne Mittel, um dies irgendwie zu automatisieren?

Figur 3

Und dann kommt ein Werkzeug wie ein statischer Code-Analysator ins Spiel. Der statische Analysator ist ein Werkzeug zum Erkennen von Fehlern im Quellcode von Programmen. Der Vorteil dieses Tools gegenüber der manuellen Überprüfung ist folgender:

  • Sie verbringen fast keine Zeit damit, nach dem Ort des Fehlers zu suchen (zumindest ist es definitiv schneller als das Suchen nach dem fehlgeschlagenen Kopieren und Einfügen mit Ihren Augen).
  • wird nicht müde, im Gegensatz zu einer Person, die nach einiger Zeit der Suche Ruhe braucht;
  • kennt viele Fehlermuster, die einer Person möglicherweise nicht einmal bewusst sind;
  • verwendet Technologien wie: Datenflussanalyse, symbolische Ausführung, Mustervergleich und andere;
  • ermöglicht es Ihnen, regelmäßig und jederzeit zu analysieren;
  • usw.

Die Verwendung eines statischen Code-Analysators ersetzt oder bricht natürlich keine Codeüberprüfungen ab. Codeüberprüfungen werden jedoch produktiver und nützlicher. Sie können sich darauf konzentrieren, Fehler auf hoher Ebene zu finden, Wissen zu übertragen, anstatt müde zu sein, den Code auf der Suche nach Tippfehlern zu lesen.

Wenn Sie mehr darüber lesen möchten, empfehle ich den folgenden Artikel sowie einen Artikel über die in PVS-Studio verwendeten Technologien .

PVS-Studio C # für Linux / macOS


Derzeit portieren wir unseren C # -Analysator auf .NET Core und entwickeln aktiv ein Plug-In für den IDE Rider.

Wenn Sie interessiert sind, können Sie sich für einen Betatest anmelden, indem Sie das Formular auf dieser Seite ausfüllen . Installationsanweisungen werden an Ihre E-Mail gesendet (keine Sorge, es ist sehr einfach) sowie eine Lizenz zur Verwendung des Analysegeräts.

So sieht Rider mit dem PVS-Studio-Plugin aus:

Bild 4

Ein bisschen Empörung


Ich möchte sagen, dass es an einigen Stellen sehr schwierig war, den Quellcode von Nethermind zu lesen, da darin Zeilen mit einer Länge von 300-500 Zeichen ganz normal sind. Ja, es ist der gesamte Code ohne Formatierung in einer Zeile. Und diese Zeilen enthalten zum Beispiel mehrere ternäre Operatoren und logische Operatoren, und dort ist nichts. Im Allgemeinen Genuss ab der letzten Saison des Thronspiels.

Ich werde ein wenig erklären, um den Umfang zu verstehen. Ich habe einen UltraWide-Monitor mit einer Länge von ca. 82 Zentimetern. Wenn Sie die IDE im Vollbildmodus öffnen, passt sie auf ca. 340 Zeichen. Das heißt, die Zeilen, über die ich spreche, passen nicht einmal. Wenn Sie sehen möchten, wie es aussieht, habe ich Links zu Dateien auf GitHub hinterlassen:

Beispiel 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}");
    }
}

Dateiverknüpfungsbeispiel

2

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

Link zur Datei

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);
    ...     
}

Link zur Datei

Stellen Sie sich nun vor, dass in einem solchen Abschnitt ein Fehler auftritt. Ist es angenehm, danach zu suchen? Ich bin mir sicher, dass dies nicht der Fall ist, und jeder versteht, dass es unmöglich ist, so zu schreiben. Übrigens gibt es in diesem Projekt einen ähnlichen Ort mit einem Fehler.

Validierungsergebnisse


Bedingungen, die 0 nicht mögen


Bedingung 1

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

PVS-Studio Warnung: V3106 Möglicherweise ist der Index nicht gebunden. Der '0'-Index zeigt über die gebundenen' Bytes 'hinaus. Nethermind.Network ReceiptsMessageSerializer.cs 50

Um einen Fehler zu bemerken, betrachten Sie den Fall, in dem die Anzahl der Elemente im Array 0 beträgt. Dann ist die Bedingung bytes.Length == 0 erfüllt und beim Zugriff auf das Array-Element wird eine indexOutOfRangeException vom Typ 0 ausgelöst .

Sie wollten diese Methode sofort beenden, wenn das Array leer ist oder das 0-Element einem bestimmten Wert entspricht, aber es scheint, dass sie versehentlich "||" verwechselt haben. mit "&&". Ich schlage vor, dieses Problem wie folgt zu beheben:

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

Bedingung 2

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

PVS-Studio Warnung: V3106 Möglicherweise ist der Index nicht gebunden. Der '0'-Index zeigt über die gebundene' typesInGroup'-Grenze hinaus. Nethermind.Runner EthereumStepsManager.cs 70

Eine ähnliche Situation wie oben beschrieben tritt auf. Wenn die Anzahl der Elemente in typesInGroup gleich 0 ist, wird beim Zugriff auf das 0-Element eine Ausnahme vom Typ IndexOutOfRangeException ausgelöst .

Nur in diesem Fall verstehe ich nicht, was der Entwickler wollte. Anstelle von typesInGroup [0] müssen Sie höchstwahrscheinlich null schreiben .

Fehler oder unvollendete Optimierung?


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 Warnung: V3102 Verdächtiger Zugriff auf das Element des Objekts 'currentLevel.BlockInfos' durch einen konstanten Index innerhalb einer Schleife. Nethermind.Blockchain BlockTree.cs 895

Auf den ersten Blick ist der Fehler offensichtlich - die Schleife angestrebt wird Iterieren über die currentLevel.BlockInfos Elemente , aber beim Zugriff auf currentLevel.BlockInfos [i], sie schrieb currentLevel.BlockInfos [0] . Wir setzen 0 auf i und die Mission ist abgeschlossen. Aber beeil dich nicht, lass es uns herausfinden.

Nun, wir Länge Zugang der BlockHash des Nullelement. Wenn es gleich currentHash ist , nehmen wir aus currentLevel.BlockInfos alle Elemente, die nicht gleich sindcurrentHash , schreibe darauf und verlasse die Schleife. Es stellt sich heraus, dass der Zyklus überflüssig ist.

Ich denke, dass es früher einen Algorithmus gab, den sie mit linq ändern / optimieren wollten , aber etwas ist schief gelaufen. In dem Fall, in dem die Bedingung falsch ist, erhalten wir bedeutungslose Iterationen.

Übrigens, wenn der Entwickler, der dies geschrieben hat, den inkrementellen Analysemodus verwenden würde, würde er sofort erkennen, dass etwas falsch gewesen wäre und alles schnell behoben hätte. Im Moment würde ich den Code folgendermaßen umschreiben:

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

Dereferenzierung der Nullreferenz


Dereferenzierung 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 Warnung: V3095 Das Objekt '_logger' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 118, 118. Nethermind.Wallet DevKeyStoreWallet.cs 118

Fehler in der falschen Reihenfolge. Zuerst gibt es einen Appell an _logger.IsDebug und erst danach ist _logger geprüft für null . Dementsprechend wird in dem Fall , wenn _logger ist null , wir eine Ausnahme vom Typ erhalten Nullreferenceexception .

Dereferenzierung 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 Warnung: V3095 Das Objekt '_enode' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 55, 56. Nethermind.JsonRpc AdminModule.cs 55 Der

Fehler ist vollständig der gleiche wie oben beschrieben, nur dass diesmal der Schuldige _enode ist .

Ich möchte hinzufügen, dass Sie, wenn Sie vergessen haben, nach Null zu suchen, dies nur dann merken, wenn Ihr Programm abstürzt. Der Analysator wird Sie daran erinnern und alles wird gut.

Unser Lieblings-Copy-Paste


Fall 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;
}

PVS-Studio Warnung: V3001 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'a.s2 == b.s2'. Nethermind.Dirichlet.Numerics UInt256.cs 1154

Hier wird der gleiche Zustand zweimal überprüft:

a.s2 == b.s2

Da die Parameter a und b ein Feld s3 haben , gehe ich davon aus, dass wir beim Kopieren einfach vergessen haben, s2 in s3 zu ändern .

Es stellt sich heraus, dass die Parameter häufiger gleich sind als vom Autor des Codes erwartet. Gleichzeitig denken einige Entwickler, dass sie dies nicht tun, und beginnen, an einem völlig anderen Ort nach einem Fehler zu suchen, wobei sie viel Energie und Nerven aufwenden.

Fehler in Vergleichsfunktionen sind übrigens in der Regel ein Klassiker. Anscheinend beziehen sich Programmierer, die solche Funktionen für einfach halten, sehr beiläufig und unaufmerksam auf ihr Schreiben. Beweis . Wenn du das weißt, sei jetzt vorsichtig :)!

Fall 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 Warnung: V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. Nethermind.BeaconNode BeaconNodeFacade.cs 177

für einen beliebigen Wert der acceptedLocally Variable, der kehrt Verfahren das gleiche. Es ist schwer zu sagen, ob es ein Fehler ist oder nicht. Angenommen, ein Programmierer hat eine Zeile kopiert und vergessen, StatusCode zu ändern. Wenn Sie auf etwas anderes zugreifen , ist dies ein echter Fehler. Darüber hinaus verfügt der StatusCode über einen InternalError und InvalidRequest . Aber das Code-Refactoring ist wahrscheinlich schuld und wir kümmern uns sowieso nicht um den akzeptierten lokalen WertIn diesem Fall wird der Zustand zum Ort, an dem Sie sitzen und überlegen, ob es sich um einen Fehler handelt oder nicht. In jedem Fall ist dieser Fall äußerst unangenehm.

Fall 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 Warnung: V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. Nethermind.Overseer.Test TestBuilder.cs 46

Und wieder kümmern wir uns nicht um die Überprüfung, weil wir am Ende das Gleiche bekommen. Und wieder sitzen wir und leiden und denken, aber was wollte der Entwickler hier schreiben? Eine Zeitverschwendung, die durch statische Analyse und sofortige Korrektur eines solchen mehrdeutigen Codes hätte vermieden werden können.

Fall 4

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

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

PVS-Studio Warnung: V3021 Es gibt zwei if-Anweisungen mit identischen bedingten Ausdrücken. Die erste 'if'-Anweisung enthält die Methodenrückgabe. Dies bedeutet , dass die zweite ‚if‘ Anweisung ist sinnlos Nethermind.Network.Benchmark InFlowBenchmarks.cs 55

Drücken Sie einfach Strg + V wieder . Wir entfernen den überschüssigen Scheck und alles ist in Ordnung. Ich bin sicher , dass , wenn eine andere Bedingung hier wichtig waren, dann alle in einem schreiben würde , wenn durch den logischen Operator I.

Fall 5

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

PVS-Studio Warnung: V3030 Wiederkehrende Prüfung. Die Bedingung '_logger.IsInfo' wurde bereits in Zeile 242 überprüft. Nethermind.Synchronization SyncServer.cs 244

Auf die gleiche Weise wie im vierten Fall wird eine zusätzliche Prüfung durchgeführt. Der Unterschied besteht jedoch darin, dass _logger nicht nur eine Eigenschaft hat, sondern beispielsweise auch ' bool IsError {get; } '. Daher sollte der Code wahrscheinlich folgendermaßen aussehen:

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

Nun, oder alle Fehler des Code-Refactorings und nur eine Überprüfung sind nicht mehr erforderlich.

Fall 6

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

PVS-Studio- Warnung : V3022 Der Ausdruck 'missingParamsCount! = 0' ist immer wahr. Nethermind.JsonRpc JsonRpcService.cs 127

Überprüfen Sie die Bedingung (missingParamsCount! = 0). Wenn sie wahr ist, berechnen wir sie erneut und weisen das Ergebnis einer Variablen zu. Stimmen Sie zu, dass dies eine ziemlich originelle Art ist, wahr zu schreiben.

Verwirrende Prüfung


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++;
    }

    ...
  }
  ...
}

PVS-Studio- Warnung : V3022 Der Ausdruck 'i == 0' ist immer falsch. Nethermind.Synchronization BlockDownloader.cs 192 Beginnen

wir in der richtigen Reihenfolge. Bei der Initialisierung wird der Variablen i der Wert 1 zugewiesen. Als nächstes wird die Variable nur inkrementiert, daher wird der Wert immer an die Funktion false übergeben .

Schauen wir uns nun HandleAddResult an :

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

Hier interessiert uns isFirstInBatch . Nach dem Namen dieses Parameters zu urteilen, ist er dafür verantwortlich, ob etwas das erste in der Partei ist. Hm, zuerst. Schauen wir noch einmal oben nach und sehen, dass es 2 Aufrufe mit i gibt :

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

Vergessen Sie nicht, dass in diesem Fall die Zählung bei 1 liegt. Es stellt sich heraus, dass wir zwei Optionen haben: Entweder bedeutet "first" ein Element bei Index 1 oder unter dem Symbol 0. In jedem Fall ist i gleichzeitig gleich 1.

Es stellt sich heraus, dass Der Funktionsaufruf sollte folgendermaßen aussehen:

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

Oder so:

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

Und wieder, wenn ein Entwickler ständig einen statischen Analysator verwendet, würde er diesen Code schreiben und eine Warnung sehen, ihn schnell beheben und das Leben genießen.

Eine Priorität ??


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);
  }
}

PVS-Studio-Warnungen:

  • V3123 Vielleicht das '??' Der Bediener arbeitet anders als erwartet. Ihre Priorität ist niedriger als die Priorität anderer Betreiber im linken Teil. Nethermind.Trie TrieNode.cs 43
  • V3123 Vielleicht das '??' Der Bediener arbeitet anders als erwartet. Ihre Priorität ist niedriger als die Priorität anderer Betreiber im linken Teil. Nethermind.Trie TrieNode.cs 44

Der Analysator empfiehlt Ihnen, zu überprüfen, wie wir die Operatoren "??" verwenden, und um das Problem zu verstehen, schlage ich vor, die folgende Situation zu berücksichtigen. Wir sehen uns diese Zeile hier an:

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

MemorySizes.RefSize und MemorySizes.ArrayOverhead sind Konstanten:

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

Aus Gründen der Übersichtlichkeit schlage ich daher vor, die Zeichenfolge neu zu schreiben und ihre Werte zu ersetzen:

(8 + FullRlp?.Length ?? 20)

Angenommen, FullRlp ist null. Dann ist (8 + null) null . Als nächstes erhalten wir den Ausdruck ( null ?? 20 ), der 20 zurückgibt .

Es stellt sich heraus, dass der Wert von MemorySizes.ArrayOverhead immer zurückgegeben wird, unabhängig davon, was in MemorySizes.RefSize gespeichert ist , vorausgesetzt, FullRlp ist null . Das Fragment in der Zeile darunter ist ähnlich. Aber die Frage ist, wollte der Entwickler dieses Verhalten? Schauen wir uns die folgende Zeile an:



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

Hier wird, wie in den oben betrachteten Abschnitten, MemorySizes.RefSize zum Ausdruck hinzugefügt.

Beachten Sie jedoch , dass nach dem ersten "+" - Operator eine Klammer steht. Es stellt sich heraus, dass wir MemorySizes.RefSize einen Ausdruck hinzufügen sollten , und wenn es null ist , werden wir einen weiteren hinzufügen. Der Code sollte also so aussehen:

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);
  }
}

Auch dies ist nur eine Annahme. Wenn der Entwickler jedoch ein anderes Verhalten wünscht, sollten Sie dies explizit angeben:

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

Und dann müsste derjenige, der diesen Code liest, nicht lange darüber nachdenken, was hier passiert und was der Programmierer wollte.

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)
    {
        ...
    }
    ...
}

PVS-Studio Warnung: V3123 Vielleicht das '??' Der Bediener arbeitet anders als erwartet. Ihre Priorität ist niedriger als die Priorität anderer Betreiber im linken Teil. Nethermind.JsonRpc JsonRpcService.cs 123

Und wieder ist die Priorität der Operation "??", daher betrachten wir wie beim letzten Mal die Situation. Wir sehen uns diese Zeile hier an:

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

Angenommen, bereitgestellte Parameter sind null. Ersetzen Sie aus Gründen der Übersichtlichkeit sofort alles, was mit bereitgestellten Parametern zusammenhängt, durch null und ersetzen Sie einen zufälligen Wert anstelle von erwarteten Parametern. Länge :

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

Jetzt fällt sofort auf, dass es zwei ähnliche Prüfungen gibt, nur in einem Fall gibt es Klammern und in dem anderen. Lassen Sie uns dieses Beispiel ausführen. Zuerst erhalten wir, dass ( null ?? 0 ) 0 zurückgibt, dann 0 von 100 subtrahiert und 100 erhält:

100 + null ?? 0;

Anstatt zuerst " null ?? 0 " auszuführen und schließlich ( 100 + 0 ) zu erhalten, werden wir völlig anders.

Zuerst wird es ausgeführt ( 100 + null ) und wir erhalten null . Dann prüft es ( null ?? 0 ), was dazu führt, dass der Wert der Variablen "yingParamsCount "0 ist.

Da die nächste Bedingung darin besteht, dass" missingParamsCount "gleich Null ist, können wir davon ausgehen, dass der Entwickler dieses Verhalten gesucht hat. Und ich werde sagen, warum nicht Klammern setzen und meine Gedanken explizit ausdrücken? Im Allgemeinen ist es möglich, dass diese Überprüfung auf ein Missverständnis zurückzuführen ist, warum manchmal 0 zurückgegeben wird, und dies ist nichts weiter als eine Krücke.

Und wieder verschwenden wir Zeit, obwohl wir dies möglicherweise nicht getan haben. Verwenden Sie beim Schreiben von Code den inkrementellen Analysemodus .

Fazit


Abschließend hoffe ich, dass ich Ihnen vermitteln konnte, dass der statische Analysator Ihr Freund ist und kein böser Aufseher, der nur darauf wartet, dass Sie einen Fehler machen.

Es sollte auch beachtet werden, dass Sie bei einmaliger oder seltener Verwendung des Analysegeräts natürlich Fehler finden und einige davon sogar schnell behoben werden können, aber es gibt auch Fehler, bei denen Sie sich den Kopf zerschlagen müssen. Daher müssen Sie ständig einen statischen Analysator verwenden. Dann werden Sie viele weitere Fehler finden und diese korrigieren, sobald der Programmcode geschrieben ist und Sie genau verstehen, was Sie versuchen zu tun.

Die einfache Wahrheit ist, dass jeder Fehler macht und das ist normal. Wir alle lernen aus Fehlern, aber nur aus denen, die es bemerken und verstehen konnten. Verwenden Sie daher moderne Tools, um nach genau diesen Fehlern zu suchen, z. B. PVS-Studio. Vielen Dank für Ihre Aufmerksamkeit.



Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Nikolay Mironov. Einzeiliger Code oder Überprüfung von Nethermind mit PVS-Studio C # für Linux .

All Articles