pvs linux_使用Linux的PVS-Studio C#进行单行代码或Nethermind的检查

pvs linux

Рисунок 1

This article coincides with the beta testing start of PVS-Studio C# for Linux, as well as the plugin for Rider. For such a wonderful reason, we checked the source code of the Nethermind product using these tools. This article will cover some distinguished and, in some cases, funny errors.

本文与针对Linux的PVS-Studio C#和Rider的插件的Beta测试开始时的情况吻合。 由于如此奇妙的原因,我们使用这些工具检查了Nethermind产品的源代码。 本文将介绍一些杰出的错误,在某些情况下还包括有趣的错误。

Nethermind is a fast client for .NET Core Ethereum for Linux, Windows, MacOs. It can be used in projects when setting up Ethereum or dApps private networks. Nethermind Nethermind是适用于Linux,Windows和MacO的.NET Core以太坊的快速客户端。 设置以太坊或dApps专用网络时,可以在项目中使用它。 Nethermind open source code is available on GitHub. The project was established in 2017 and is constantly evolving. 开源代码可在GitHub上获得。 该项目成立于2017年,并且在不断发展。

介绍 (Introduction)

Do you like manual labor? For example, such as finding errors in program code. It stands to reason, it's rather tedious to read and analyze your own fragment of code or a whole project in search of a tricky bug. It's no big deal if a project is small, let's say 5,000 lines, but what if its size has already exceeded one hundred thousand or a million lines? In addition, it might be written by several developers and in some cases not in a very digestible form. What is to be done in this case? Do we really have to fall behind on sleep, have no regular meals, and spend 100% of time probing into all these endless lines in order to understand where this nasty mistake is? I doubt that you would like to do this. So what shall we do? Maybe there are modern means to somehow automate this?

你喜欢体力劳动吗? 例如,例如在程序代码中查找错误。 理所当然,阅读和分析您自己的代码片段或整个项目以查找棘手的错误非常繁琐。 如果一个项目很小,比如说5,000行,那没什么大不了的,但是如果它的规模已经超过了十万或一百万行,那该怎么办? 此外,它可能是由多个开发人员编写的,在某些情况下,其格式不是很容易理解。 在这种情况下该怎么办? 为了了解这个令人讨厌的错误在哪里,我们真的必须在睡眠上落后,没有定时进餐并且花100%的时间探索所有这些无休止的路线吗? 我怀疑您想这样做。 那我们该怎么办? 也许有现代化的方法可以以某种方式使其自动化?

Рисунок 3

Here a tool like a static code analyzer comes into play. Static analyzer is a tool for detecting defects in the source code of programs. The advantage of this tool over manual check boils down to these points:

静态代码分析器之类的工具在这里发挥作用。 静态分析器是用于检测程序源代码中的缺陷的工具。 与手动检查相比,此工具的优势归结为以下几点:

  • it almost doesn't spend your time when searching for an erroneous fragment. At least it's definitely faster than a human looking for a failed copy-paste;

    搜索错误片段几乎不会花费您的时间。 至少肯定比寻找失败的复制粘贴的人要快。
  • it doesn't get tired, unlike a person who will need rest after some time of searching;

    它不会累,不像一个人经过一段时间的搜索后需要休息;
  • it knows a lot of error patterns that a person may not even be aware of;

    它知道一个人甚至可能不知道的许多错误模式;
  • it uses technologies such as: data flow analysis, symbolic execution, pattern matching, and others;

    它使用的技术包括:数据流分析,符号执行,模式匹配等。
  • it allows you to regularly perform the analysis at any time;

    它允许您随时定期执行分析;
  • and so on.

    等等。

Of course, usage of a static code analyzer doesn't replace or obviate the need of code reviews. However, with this tool code reviews become more productive and useful. You can focus on finding high-level errors, imparting knowledge, rather than just wearily review code in search of typos.

当然,使用静态代码分析器不会替代或消除代码审查的需要。 但是,使用此工具,代码复审变得更加高效和有用。 您可以专注于发现高级错误,传授知识,而不仅是疲倦地检查代码以查找错误。

If you became interested in reading more about it, then I suggest the following article, as well as an article about the technologies used in PVS-Studio.

如果您有兴趣阅读更多有关它的内容,那么我建议您推荐以下文章 ,以及有关PVS-Studio中使用的技术的文章。

适用于Linux / macOS的PVS-Studio C# (PVS-Studio C# for Linux/macOS)

We are currently porting our C# analyzer to .NET Core, and we are also actively developing a plugin for the Rider IDE.

我们目前正在将C#分析器移植到.NET Core,并且我们也在积极开发Rider IDE插件。

If you are interested, you can sign up for beta testing by filling out the form on this page. Installation instructions will be sent to your mail (don't worry, it's very simple), as well as a license for using the analyzer.

如果您有兴趣,可以通过填写此页面上的表格来注册Beta测试。 安装说明将发送到您的邮件中(不用担心,这很简单),以及使用分析仪的许可证。

This is how Rider looks like with the PVS-Studio plugin:

这是PVS-Studio插件的Rider外观:

Picture 4

有点生气 (A bit of outrage)

I'd like to mention that some fragments of the Nethermind code were difficult to perceive, as lines of 300-500 characters are normal for it. That's it, code in one line without formatting. For example, these lines might contain both several ternary operators, and logical operators, they got everything there. It's just as 'delightful' as the last Game of Thrones season.

我想提一下,Nethermind代码的某些片段很难理解,因为300-500个字符的行是正常的。 就是这样,无需格式化即可在一行中进行编码。 例如,这些行可能同时包含几个三元运算符和逻辑运算符,它们在那里得到了所有信息。 就像上一季《权力的游戏》一样“令人愉快”。

Let me make some clarifications for you to become aware of the scale. I have an UltraWide monitor, which is about 82 centimeters (32 inches) long. Opening the IDE on it in full screen, it fits about 340 characters, that is, the lines that I'm talking about don't even fit. If you want to see how it looks, I left the links to files on GitHub:

让我澄清一下,以便您了解该规模。 我有一台UltraWide显示器,它的长度约为82厘米(32英寸)。 全屏打开IDE时,它可以容纳大约340个字符,也就是说,我所说的行甚至不合适。 如果您想看看它的外观,我将链接保留在GitHub上:

例子1 (Example 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}");
    }
}

Link to the file

链接到文件

例子2 (Example 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 to the file

链接到文件

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 to the file

链接到文件

Would it be nice to search for such an error in such a fragment? I'm sure, everyone is perfectly aware that it wouldn't be nice and one shouldn't write code in this way. By the way, there is a similar place with an error in this project.

在这样的片段中搜索这样的错误会很好吗? 我敢肯定,每个人都清楚地知道那不是很好,也不应该以这种方式编写代码。 顺便说一句,这个项目中有一个类似的地方出错。

分析结果 (Analysis results)

不喜欢0的条件 (Conditions that don't like 0)

条件1 (Condition 1)

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

PVS-Studio warning: V3106 Possibly index is out of bound. The '0' index is pointing beyond 'bytes' bound. Nethermind.Network ReceiptsMessageSerializer.cs 50

PVS-Studio警告: V3106可能索引超出范围。 索引“ 0”指向“字节”界限之外。 Nethermind.Network收据MessageSerializer.cs 50

In order to take a close look at the error, let's consider the case with the 0 number of elements in the array. Then the bytes.Length == 0 condition will be true and when accessing the array element the IndexOutOfRangeException type exception will occur.

为了仔细研究错误,让我们考虑数组中元素数量为0的情况。 然后bytes.Length == 0条件将为true,并且在访问数组元素时,将发生IndexOutOfRangeException类型异常。

Perhaps, the code author wanted to exit the method right away if the array is empty or the 0 element is equal to a certain value. Yet it seems like the author got "||" and "&&" confused. I suggest fixing this problem as follows:

也许,如果数组为空或0元素等于某个值,代码作者可能想立即退出该方法。 但是似乎作者得到了“ ||” 与“ &&”混淆。 我建议按以下方式解决此问题:

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

条件2 (Condition 2)

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

PVS-Studio warning: V3106 Possibly index is out of bound. The '0' index is pointing beyond 'typesInGroup' bound. Nethermind.Runner EthereumStepsManager.cs 70

PVS-Studio警告: V3106可能索引超出范围。 索引“ 0”指向“ typesInGroup”的界限之外。 Nethermind.Runner以太坊StepsManager.cs 70

Here we have the case, similar to the above. If the number of elements in typesInGroup is 0, then when accessing the 0 element, an exception of IndexOutOfRangeException type will occur.

在这里,我们遇到了类似上面的情况。 如果typesInGroup中的元素为0,则在访问0元素时,将发生IndexOutOfRangeException类型的异常。

But in this case I don't understand what the developer wanted. Most likely, null has to be written instead of typesInGroup[0].

但是在这种情况下,我不了解开发人员的需求。 最有可能的是,必须写入null而不是typesInGroup [0]。

错误或优化不完善? (An error or an uncomplete optimization?)

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 Suspicious access to element of 'currentLevel.BlockInfos' object by a constant index inside a loop. Nethermind.Blockchain BlockTree.cs 895

PVS-Studio警告: V3102通过循环内的常量索引可疑访问'currentLevel.BlockInfos'对象的元素。 冥想区块链BlockTree.cs 895

At first glance, the error is obvious — the loop is supposed to iterate over the currentLevel.BlockInfos elements. Nevertheless, authors wrote currentLevel.BlockInfos[0] instead of currentLevel.BlockInfos[i] when accessing it. So we change 0 for i to complete our mission. No such luck! Let's get this over.

乍一看,错误是显而易见的-循环应该在currentLevel.BlockInfos元素上进行迭代。 但是,作者在访问它时写了currentLevel.BlockInfos [0]而不是currentLevel.BlockInfos [i] 。 所以我们改变0 来完成我们的使命。 没有这种运气! 让我们解决这个问题。

At this point we access BlockHash of the zeroth element Length times. If it's equal to currentHash, we take all elements not equal to currentHash from currentLevel.BlockInfos. Then we write them in this very currentLevel.BlockInfos and exit the loop. It turns out that the loop is redundant.

此时,我们访问第零个元素的Length倍的BlockHash 。 如果等于currentHash ,则从currentLevel.BlockInfos中获取所有不等于currentHash的元素。 然后,将它们写入当前的Level.BlockInfos并退出循环。 事实证明,该循环是多余的。

I think that earlier there was an algorithm that the author decided to change/optimize using linq, but something went wrong. Now, in the case when the condition is false, we get meaningless iterations.

我认为以前有一种算法,作者决定使用linq进行更改/优化,但是出了点问题。 现在,如果条件为假,我们将得到无意义的迭代。

By the way, if the developer who had written this had used the incremental analysis mode, then he would have immediately realized that something had been wrong and would have fixed everything straight away. Given the above, I would rewrite the code like this:

顺便说一句,如果编写此文档的开发人员使用了增量分析模式 ,那么他将立即意识到出了点问题,并会立即修复所有问题。 鉴于以上所述,我将像这样重写代码:

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

取消引用空引用的情况 (Cases of dereferencing null reference)

解除引用1 (Dereference 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 The '_logger' object was used before it was verified against null. Check lines: 118, 118. Nethermind.Wallet DevKeyStoreWallet.cs 118

PVS-Studio警告: V3095在验证为空之前使用了'_logger'对象。 检查行:118,118。Nethermind.Wallet DevKeyStoreWallet.cs 118

The error is in the wrong sequence. First _logger.IsDebug is accessed followed by the _logger check for null. Accordingly, if _logger is null, we'll get the NullReferenceException.

错误的顺序错误。 首先访问_logger.IsDebug ,然后通过_logger检查是否为空。 因此,如果_loggernull ,我们将获得NullReferenceException。

取消引用2 (Dereference 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 The '_enode' object was used before it was verified against null. Check lines: 55, 56. Nethermind.JsonRpc AdminModule.cs 55

PVS-Studio警告: V3095在验证空值之前使用了'_enode'对象。 检查行:55、56。Nethermind.JsonRpc AdminModule.cs 55

The error is completely similar to the one described above, except for this time _enode is to blame here.

该错误与上述错误完全类似,只是这次是_enode的错误

I might add, that if you forget to check something for null, you'll probably be reminded only when the program crashes. The analyzer will remind you of this and everything will be fine.

我可能会补充说,如果您忘记检查某些内容是否为null,则只有在程序崩溃时才会提醒您。 分析仪会提醒您这一点,一切都会好起来的。

我们深爱的复制粘贴 (Our dearly beloved Copy-Paste)

片段N1 (Fragment N1 )

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 warning: V3001 There are identical sub-expressions 'a.s2 == b.s2' to the left and to the right of the '&&' operator. Nethermind.Dirichlet.Numerics UInt256.cs 1154

PVS-Studio警告: V3001 '&&'运算符的左侧和右侧有相同的子表达式'a.s2 == b.s2'。 Nethermind.Dirichlet.Numerics UInt256.cs 1154

Here the same condition is checked twice:

在此,两次检查相同的条件:

a.s2 == b.s2

Since a and b parameters have the s3 field, I assume the developer simply forgot to change s2 for s3 when copying.

由于ab参数具有s3字段,因此我假设开发人员只是在复制时忘记将s2更改为s3

It turns out that the parameters will be equal more often than expected by this fragment author. At the same time, some developers suppose that they can't write something like this and they start looking for an error in a completely different place, wasting a lot of energy and nerves.

事实证明,参数相等的次数比该片段作者期望的要多。 同时,一些开发人员认为他们无法编写这样的内容,因此他们开始在完全不同的地方寻找错误,这浪费了很多精力和神经。

By the way, errors in comparison functions are generally a classic. Apparently, programmers, considering such functions simple, treat writing their code very casually and inattentively. Proof. Now you know about this, so stay alert :)!

顺便说一下,比较函数中的错误通常是经典的。 显然,程序员认为此类函数很简单,因此非常随意且不专心地编写代码。 证明 。 现在您知道了,所以保持警惕:)!

片段N2 (Fragment N2 )

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 The 'then' statement is equivalent to the 'else' statement. Nethermind.BeaconNode BeaconNodeFacade.cs 177

PVS-Studio警告: V3004'then '语句等效于'else'语句。 Nethermind.BeaconNode BeaconNodeFacade.cs 177

For any value of the acceptedLocally variable, the method returns the same. Tough to tell, whether it is an error or not. Suppose a programmer copied a line and forgot to change StatusCode.Success for something else — in this way, it's a real error. Moreover, StatusCode has InternalError and InvalidRequest. Perhaps, it's all the fault of code refactoring and the acceptedLocally value doesn't matter. This way, the condition makes us sit around thinking whether it's an error or not. So in any case, this case is extremely nasty.

对于acceptedLocally变量的任何值,该方法将返回相同的值。 很难说,这是否是一个错误。 假设程序员复制了一行,而忘记更改StatusCode 。成功完成其他操作—这样,这是一个真正的错误。 此外, StatusCode具有InternalErrorInvalidRequest 。 也许是代码重构的全部错误, acceptedLocally值无关紧要。 通过这种方式,条件使我们无处不在地思考这是否是错误。 因此,无论如何,这种情况非常讨厌。

片段N3 (Fragment N3 )

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 The 'then' statement is equivalent to the 'else' statement. Nethermind.Overseer.Test TestBuilder.cs 46

PVS-Studio警告: V3004'then '语句等效于'else'语句。 Nethermind.Overseer.Test TestBuilder.cs 46

There we go again not paying much attention to the check, as we get the same result. So we're wondering and racking our brains thinking about the developer's intentions. A waste of time that could have been avoided by using static analysis and immediately fixing such ambiguous code.

由于我们得到了相同的结果,因此我们再次没有对检查进行过多的关注。 因此,我们想知道并绞尽脑汁考虑开发人员的意图。 通过使用静态分析并立即修复此类歧义代码,可以避免浪费时间。

片段N4 (Fragment N4)

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 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless Nethermind.Network.Benchmark InFlowBenchmarks.cs 55

PVS-Studio警告: V3021有两个带有相同条件表达式的'if'语句。 第一个'if'语句包含方法return。 这意味着第二个“ if”语句是毫无意义的Nethermind.Network.Benchmark InFlowBenchmarks.cs 55

Someone accidently pressed Ctrl+V one extra time. We remove the excess check and everything looks fine. I'm sure that if another condition were important here, then everything would be written in one if block using the AND logical operator.

有人不小心多按了Ctrl + V一次。 我们删除了多余的支票,一切看起来都很好。 我敢肯定,如果在这里另一个条件很重要,那么所有的东西都将使用AND逻辑运算符写在一个if块中。

N5片段 (Fragment N5)

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

PVS-Studio warning: V3030 Recurring check. The '_logger.IsInfo' condition was already verified in line 242. Nethermind.Synchronization SyncServer.cs 244

PVS-Studio警告: V3030定期检查。 在行242中已经验证了“ _logger.IsInfo”条件。Nethermind.Synchronization SyncServer.cs 244

As in the fourth case, an extra check is performed. However, the difference is that not only does _logger have only one property, it also has, for example, 'bool IsError { get; }'. Therefore, the code should probably look like this:

与第四种情况一样,将执行额外的检查。 但是,区别在于_logger不仅仅具有一个属性,而且还具有例如' bool IsError {get; } '。 因此,代码可能看起来像这样:

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

Or maybe pesky refactoring is responsible for it and one check is no longer needed.

或者也许讨厌的重构是它的责任,并且不再需要检查。

片段N6 (Fragment N6)

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

PVS-Studio warning: V3022 Expression 'missingParamsCount != 0' is always true. Nethermind.JsonRpc JsonRpcService.cs 127

PVS-Studio警告: V3022表达式'missingParamsCount!= 0'始终为true。 Nethermind.JsonRpc JsonRpcService.cs 127

Here we check the condition (missingParamsCount != 0) and if it's true, then again we calculate it and assign the result to the variable. Agree that this is a fairly original way to write true.

在这里,我们检查条件(missingParamsCount!= 0),如果为true,则再次对其进行计算并将结果分配给变量。 同意这是写true的相当原始的方式。

令人困惑的支票 (Confusing check)

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 warning: V3022 Expression 'i == 0' is always false. Nethermind.Synchronization BlockDownloader.cs 192

PVS-Studio警告: V3022表达式'i == 0'始终为false。 Nethermind.Synchronization BlockDownloader.cs 192

Let's start from the beginning. When initializing, the variable i is assigned the value 1. Further, the variable is only incremented, therefore, false will always be passed to the function.

让我们从头开始。 初始化时,变量i被赋值为1。此外,变量仅递增,因此,总是将false传递给函数。

Now let's look at HandleAddResult:

现在让我们看一下HandleAddResult

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

Here we are interested in isFirstInBatch. Judging by the name of this parameter, it is responsible for whether something is the first in the line. Hm, first. Let's look again above and see that there are 2 calls using i:

在这里,我们对isFirstInBatch感兴趣 根据此参数的名称判断,它负责确定行中是否有第一行。 嗯,首先。 让我们在上面再次查看,发现使用i有2个调用:

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

Don't forget that the countdown in this case comes from 1. It turns out that we have 2 options: either «first» means an element under index 1, or under index 0. But in any case, i will be equal to 1.

不要忘记这种情况下的倒数是1。事实证明,我们有2个选择:«first»表示索引1或索引0之下的元素。但是无论如何, 等于1 。

It follows that the function call should look like this:

因此,函数调用应如下所示:

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

Or in this way:

或以这种方式:

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

And again, if the developer constantly used a static analyzer, then he would write this code and see the warning, he would quickly fix it and enjoy life.

再者,如果开发人员经常使用静态分析器,那么他将编写此代码并查看警告,他将快速修复它并享受生活。

优先 ?? (Priority ??)

情况1 (Case 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 warnings:

PVS-Studio警告:

  • V3123 Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. Nethermind.Trie TrieNode.cs 43

    V3123也许是“ ??” 操作员的工作方式与预期不同。 它的优先级低于左侧其他运营商的优先级。 Nethermind.Trie TrieNode.cs 43
  • V3123 Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. Nethermind.Trie TrieNode.cs 44

    V3123也许是“ ??” 操作员的工作方式与预期不同。 它的优先级低于左侧其他运营商的优先级。 Nethermind.Trie TrieNode.cs 44

The analyzer advises us to check how we use the "??" operators. In order to understand what the problem is, I propose to consider the following situation. Look at this line here:

分析仪建议我们检查如何使用“ ??” 操作员。 为了了解问题所在,我建议考虑以下情况。 在这里看这行:

(MemorySizes.RefSize + FullRlp?.Length ?? MemorySizes.ArrayOverhead)
MemorySizes.RefSize and MemorySizes.RefSize MemorySizes.ArrayOverhead are constants. MemorySizes.ArrayOverhead是常量。
public static class MemorySizes
{
    ...
    public const int RefSize = 8;
    public const int ArrayOverhead = 20;
    ...
}

Therefore, for clarity, I suggest rewriting the line, substituting their values:

因此,为清楚起见,我建议重写该行,并替换其值:

(8 + FullRlp?.Length ?? 20)

Now suppose FullRlp is null. Then (8 + null) will be null. Next, we get the expression (null ?? 20), which will return 20.

现在假设FullRlpnull 。 然后(8 + null)将为null。 接下来,我们获得表达式( null ?? 20 ),它将返回20。

As a result, in case if FullRlp is null, the value from MemorySizes.ArrayOverhead will always be returned regardless of what is stored in MemorySizes.RefSize. The fragment on the line below is similar.

其结果是,在如果FullRlp是的情况下,从MemorySizes.ArrayOverhead值将始终不管是什么存储在MemorySizes.RefSize返回 下一行的片段类似。

But the question is, did the developer want this behavior? Let's look at the following line:

但是问题是,开发人员是否想要这种行为? 让我们看一下以下行:

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

Same as in the fragments above, MemorySizes.RefSize is added to the expression, but note that after the first "+" operator there is a bracket.

与上面的片段相同,将MemorySizes.RefSize添加到表达式中,但是请注意,在第一个“ +”运算符之后有一个括号。

It turns out that it is MemorySizes.RefSize which we should add some expression to, and if it is null, then we should add another one. So the code should look like this:

原来是MemorySizes.RefSize ,我们应该向其中添加一些表达式,如果为null ,则应该添加另一个表达式。 因此,代码应如下所示:

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

Again, this is only an assumption, however, if the developer wanted different behavior, then one should explicitly indicate this:

同样,这只是一种假设,但是,如果开发人员想要不同的行为,则应该明确指出这一点:

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

In doing so, the one who reads this code wouldn't have to delve into it for a long time puzzling out what is happening here and what the developer wanted.

这样一来,阅读此代码的人就不必长时间研究它,就可以弄清楚这里正在发生什么以及开发人员想要什么。

情况二 (Case 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 warning: V3123 Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. Nethermind.JsonRpc JsonRpcService.cs 123

PVS-Studio警告: V3123也许是“ ??” 操作员的工作方式与预期不同。 它的优先级低于左侧其他运营商的优先级。 Nethermind.JsonRpc JsonRpcService.cs 123

Here again we deal with the priority of the operation "??". Therefore we'll consider this case. Look at this line:

在这里,我们再次处理操作“ ??”的优先级。 因此,我们将考虑这种情况。 看这行:

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

Suppose that providedParameters is null, then for clarity, let's replace everything related to providedParameters with null straight away, and substitute a random value instead of expectedParameters.Length:

假设providedParameters ,那么清晰,让我们更换与使用空providedParameters马上的一切,并替换随机值,而不是expectedParameters.Length:

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

Now it's immediately noticeable that there are two similar checks, but unlike the first case there are no parentheses in the second one. Let's run this example. First we get that (null ?? 0) will return 0, then subtract 0 from 100 and get 100:

现在,可以立即注意到有两个类似的检查,但是与第一种情况不同,第二种没有括号。 让我们运行这个例子。 首先我们得到( null ?? 0 )将返回0,然后从100中减去0并得到100:

100 + null ?? 0;

Now instead of performing "null ?? 0" and getting (100 + 0), we'll get a completely different result.

现在,我们将执行完全不同的结果,而不是执行“ null ?? 0 ”并获取( 100 + 0 )。

First (100 + null) will be executed resulting in null. Then (null ?? 0) is checked leading to the fact that the value of the missingParamsCount variable will be 0.

首先执行( 100 + null ),将导致null 。 然后检查( null ?? 0 ),导致missingParamsCount变量的值将为0。

Since there's a condition that further checks whether missingParamsCount is not equal to null, we can assume that the developer sought exactly this behavior. Let me tell you something — why not put parentheses and clearly express your intentions? Perhaps, this check was due to misunderstanding why in some cases 0 is returned. This is nothing more than a kludge.

由于有一个条件可以进一步检查missingParamsCount是否不等于null,因此我们可以假定开发人员正正是在寻求这种行为。 让我告诉您一些事情-为什么不加上括号并清楚地表达您的意图? 也许,该检查是由于误解,为什么在某些情况下会返回0。 这只不过是一个k。

And again, we are wasting time, although we might not have done this, if only the developer had used the incremental analysis mode when writing code.

再一次,我们浪费时间,尽管如果开发人员在编写代码时使用了增量分析模式 ,尽管我们可能没有这样做。

结论 (Conclusion)

In conclusion, I hope that I was able to convey that the static analyzer is your friend, and not an evil overseer who is just waiting for you to make a mistake.

最后,我希望我能传达出静态分析器是您的朋友,而不是邪恶的监督者,他只是在等您犯错。

It should also be noted that by using an analyzer once, or rarely using it, you will still find errors and some of them will even be quickly fixed, but there will also be ones that you need to smash your head at. Therefore, you need to use a static analyzer regularly. Then you will find much more errors and fix them right when writing the code. Doing so, you'll be completely aware of what you are trying to do.

还应注意,一次使用分析仪,或很少使用分析仪,您仍会发现错误,并且其中一些错误甚至会很快得到解决,但是也有一些错误需要您的注意。 因此,您需要定期使用静态分析仪。 然后,您将发现更多错误,并在编写代码时立即进行纠正。 这样做,您将完全知道您要做什么。

The simple truth is that everyone makes mistakes and that's normal. We all learn from mistakes, but only from those that we notice and delve into. For this reason, use modern tools to search for these very errors, for example — PVS-Studio. Thank you for your attention.

一个简单的事实是,每个人都会犯错误,这很正常。 我们都从错误中学习,但只能从我们注意到并深入研究的错误中学习。 因此,请使用现代工具搜索这些错误,例如PVS-Studio。 感谢您的关注。

翻译自: https://habr.com/en/company/pvs-studio/blog/504316/

pvs linux

  • 0
    点赞
  • 1
    收藏
    觉得还不错? 一键收藏
  • 0
    评论

“相关推荐”对你有帮助么?

  • 非常没帮助
  • 没帮助
  • 一般
  • 有帮助
  • 非常有帮助
提交
评论
添加红包

请填写红包祝福语或标题

红包个数最小为10个

红包金额最低5元

当前余额3.43前往充值 >
需支付:10.00
成就一亿技术人!
领取后你会自动成为博主和红包主的粉丝 规则
hope_wisdom
发出的红包
实付
使用余额支付
点击重新获取
扫码支付
钱包余额 0

抵扣说明:

1.余额是钱包充值的虚拟货币,按照1:1的比例进行支付金额的抵扣。
2.余额无法直接购买下载,可以购买VIP、付费专栏及课程。

余额充值