pvs-stdio ue4_通过PVS-Studio分析仪进行RunUO检查

pvs-stdio ue4

This article covers the check of the RunUO project using the static PVS-Studio analyzer. RunUO is the emulator of server software for Ultima Online, the game that once won the hearts of many MMORPG fans.

本文介绍了使用静态PVS-Studio分析仪对RunUO项目的检查。 RunUO是Ultima Online的服务器软件的仿真器，该游戏曾经赢得了许多MMORPG粉丝的关注。

介绍 (Introduction)

RunUO is a server software emulator for MMORPG Ultima Online. The goal of this project is to create stable software that will be able to compete with the official servers of EA Games. RunUO was created back in 2002, but is relevant and actively used to this day. RunUO是MMORPG Ultima Online的服务器软件模拟器。 该项目的目标是创建稳定的软件，使其能够与EA Games的官方服务器竞争。 RunUO早在2002年就创建了，但是直到今天它还是很重要并且很活跃。

The purpose of this project review is to popularize the topic of static analysis. We check various projects — games (example), libraries (example), messengers (example), browsers (example) and more (example, example, example) to catch an eye of the most diverse audience. With these articles, we try to draw attention to the importance of using static analysis in development process. Static analysis makes the code more reliable and safer. Also, with its regular use, you can find and fix bugs at the earliest stages. This saves time and efforts of developers, as no one wants to spend 50 hours seeking out an error that the analyzer can detect.

本项目审查的目的是普及静态分析主题。 我们检查各种项目-游戏( 示例 )，库( 示例 )，通讯程序( 示例 )，浏览器( 示例 )以及更多( 示例 ， 示例 ， 示例 )，以吸引最多样化的受众。 通过这些文章，我们试图提请人们注意在开发过程中使用静态分析的重要性。 静态分析使代码更可靠，更安全。 此外，通过定期使用，您可以在最早的阶段发现并修复错误。 由于没有人愿意花费50个小时来找出分析仪可以检测到的错误，因此节省了开发人员的时间和精力。

We also help the open-source community. By posting articles with found errors, we contribute to the development of open-source community. However, we don't break all warnings down in the articles. As for this article, some warnings seemed too ordinary to get into it, some appeared to be false positives and so on. Therefore, we're ready to provide with a free license for working with open source projects. Besides, what we considered uninteresting might seem quite intriguing for developers of open source project under the check. After all, project developers know best which problems are most critical.

我们还帮助开源社区。 通过发布发现错误的文章，我们为开源社区的发展做出了贡献。 但是，我们不会在文章中细分所有警告。 至于本文，有些警告似乎太普通而无法理解，有些似乎是误报，等等。 因此，我们准备为使用开源项目提供免费许可证 。 此外，对于那些接受检查的开源项目的开发人员来说，我们认为无趣的内容似乎很有趣。 毕竟，项目开发人员最清楚哪些问题最关键。

分析仪报告中最醒目的代码片段 (Most striking code fragments from the analyzer's report)

PVS-Studio警告： (PVS-Studio warning: )

V3010 The return value of function 'Intern' is required to be utilized. BasePaintedMask.cs 49

V3010需要使用功能“ Intern”的返回值。 BasePaintedMask.cs 49

public static string Intern( string str )
{
  if ( str == null )
    return null;
  else if ( str.Length == 0 )
    return String.Empty;

  return String.Intern( str );
}

public BasePaintedMask( string staffer, int itemid )
                            : base( itemid + Utility.Random( 2 ) )
{
  m_Staffer = staffer;

  Utility.Intern( m_Staffer );
}

The return value of the Intern() method isn't taken into account anywhere, as the analyzer points out. Maybe it's an error or redundant code.

分析器指出，在任何地方都不会考虑Intern()方法的返回值。 可能是错误或冗余代码。

PVS-Studio警告： (PVS-Studio warning: )

V3017 A pattern was detected: (item is BasePotion) || ((item is BasePotion) && ...). The expression is excessive or contains a logical error. Cleanup.cs 137

V3017检测到一个模式：(项目为BasePotion)|| ((项目为BasePotion)&& ...)。 该表达式过多或包含逻辑错误。 第137章

public static bool IsBuggable( Item item )
{
  if ( item is Fists )
    return false;

  if ( item is ICommodity || item is Multis.BaseBoat
    || item is Fish || item is BigFish
    || item is BasePotion || item is Food || item is CookableFood
    || item is SpecialFishingNet || item is BaseMagicFish
    || item is Shoes || item is Sandals
    || item is Boots || item is ThighBoots
    || item is TreasureMap || item is MessageInABottle
    || item is BaseArmor || item is BaseWeapon
    || item is BaseClothing
    || ( item is BaseJewel && Core.AOS )
    || ( item is BasePotion && Core.ML )
  {
    ....
  }
}

There are sub-expressions here that can be simplified. I'll cite them for clarity:

这里有可以简化的子表达式。 为了清楚起见，我会引用它们：

if (item is BasePotion || ( item is BasePotion && Core.ML ))

Suppose item is BasePotion = true, then the condition will be true despite Core.ML. But if item is BasePotion = false, the condition will be false, again despite the Core.ML value. In most cases, such code is simply redundant, but there are worse cases, when the developer made a mistake and wrote a wrong variable in the second sub-expression.

假设item是BasePotion = true ，则尽管有Core.ML ，但条件将为true。 但是，如果item为BasePotion = false ，那么即使使用Core.ML值，条件也将为false。 在大多数情况下，这样的代码只是多余的，但在更糟的情况下，当开发人员在第二个子表达式中犯了一个错误并写了一个错误的变量时。

PVS-Studio警告： (PVS-Studio warning: )

V3031 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions 'bPlayerOnly' and '!bPlayerOnly'. BaseCreature.cs 3005

V3031过度检查可以简化。 “ ||” 运算符被相反的表达式'bPlayerOnly'和'！bPlayerOnly'包围。 BaseCreature.cs 3005

public virtual double GetFightModeRanking( Mobile m,
                                           FightMode acqType,
                                           bool bPlayerOnly )
{
  if ( ( bPlayerOnly && m.Player ) ||  !bPlayerOnly )
  {
    ....
  }
  ....
}

This code is either redundant or erroneous. The problem is that there are different sub-expressions on different sides of '||'. If we narrow it down to this:

该代码是冗余的或错误的。 问题在于'||'的不同边有不同的子表达式。 如果我们将其范围缩小到：

if ( m.Player || !bPlayerOnly )

nothing will change.

一切都不会改变。

PVS-Studio警告： (PVS-Studio warning: )

V3001 There are identical sub-expressions 'deed is SmallBrickHouseDeed' to the left and to the right of the '||' operator. RealEstateBroker.cs 132

V3001在“ ||”的左侧和右侧有相同的子表达式“ deed is SmallBrickHouseDeed” 操作员。 RealEstateBroker.cs 132

public int ComputePriceFor( HouseDeed deed )
{
  int price = 0;

  if ( deed is SmallBrickHouseDeed ||    // <=
       deed is StonePlasterHouseDeed ||
       deed is FieldStoneHouseDeed ||
       deed is SmallBrickHouseDeed ||    // <=
       deed is WoodHouseDeed ||
       deed is WoodPlasterHouseDeed ||
       deed is ThatchedRoofCottageDeed )
      ....
}

I don't think there's anything to explain. It's another erroneous or redundant piece of code.

我认为没有什么可解释的。 这是另一段错误或冗余的代码。

PVS-Studio警告： (PVS-Studio warning: )

V3067 It is possible that 'else' block was forgotten or commented out, thus altering the program's operation logics. BaseHouse.cs 1558

V3067可能忘记或注释了“ else”块，从而改变了程序的操作逻辑。 BaseHouse.cs 1558

private void SetLockdown( Item i, bool locked, bool checkContains )
{
  if ( m_LockDowns == null )
    return;

  #region Mondain's Legacy
  if ( i is BaseAddonContainer )
    i.Movable = false;
  else
  #endregion

  i.Movable = !locked;
  i.IsLockedDown = locked;

  ....
}

It's a quite rare warning. The analyzer found it suspicious to format the code after the #endregion directive in such a way. If we don't read the code carefully, it looks like the line

这是非常罕见的警告。 分析器发现以这种方式在#endregion指令后格式化代码是可疑的。 如果我们不仔细阅读代码，则该行看起来像

i.Movable = !locked;

will execute anyway regardless of the variable i. Perhaps, authors forgot to write curly brackets here… Well, code authors should revise this fragment.

无论变量i如何，都将执行。 也许，作者忘记了在此处写大括号…嗯，代码作者应修改此片段。

PVS-Studio警告： (PVS-Studio warning: )

V3043 The code's operational logic does not correspond with its formatting. The statement is indented to the right, but it is always executed. It is possible that curly brackets are missing. Earthquake.cs 57

V3043代码的操作逻辑与其格式不符。 该语句向右缩进，但始终会执行。 大括号可能丢失。 地震57

public override void OnCast()
{
  if ( Core.AOS )
  {
    damage = m.Hits / 2;

    if ( !m.Player )
      damage = Math.Max( Math.Min( damage, 100 ), 15 );
      damage += Utility.RandomMinMax( 0, 15 );            // <=

  }
  else
  {
    ....
  }
}

This code probably lacks curly brackets. We can conclude it because of strange code formatting in the if ( !m.Player ) body.

此代码可能缺少大括号。 我们可以得出结论，因为if(！m.Player)主体中的代码格式奇怪。

PVS-Studio警告： (PVS-Studio warning: )

V3083 Unsafe invocation of event 'ServerStarted', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. EventSink.cs 921

V3083事件'ServerStarted'的不安全调用，可能会发生NullReferenceException。 请考虑在调用事件之前将事件分配给局部变量。 EventSink.cs 921

public static void InvokeServerStarted()
{
  if ( ServerStarted != null )
    ServerStarted();
}

In this method, unsafe call of the RefreshStarted event handler is used, as the analyzer indicates.

在这种方法中，使用了不安全的RefreshStarted事件处理程序调用，正如分析器所指出的那样。

Why is it dangerous? Imagine this situation. The ServerStarted event has only one subscriber. In the moment between checking for null and directly calling the ServerStarted() event handler, someone unsubscribed from the event in another thread. This will result in NullReferenceException.

为什么有危险？ 想象一下这种情况。 ServerStarted事件只有一个订阅者。 在检查null和直接调用ServerStarted()事件处理程序之间的那一刻，有人在另一个线程中取消订阅该事件。 这将导致NullReferenceException 。

The easiest way to prevent this situation is to ensure that the event is safely called with the '?.' operator:

防止这种情况的最简单方法是确保使用“？”安全地调用该事件。 操作员：

public static void InvokeServerStarted()
{
  ServerStarted?.Invoke();
}

PVS-Studio警告： (PVS-Studio warning: )

V3054 Potentially unsafe double-checked locking. Use volatile variable(s) or synchronization primitives to avoid this. Item.cs 1624

V3054潜在的不安全的双重检查锁定。 使用易失性变量或同步原语可以避免这种情况。 Item.cs 1624

private Packet m_RemovePacket;
....
private object _rpl = new object();
public Packet RemovePacket
{
  get
  {
    if (m_RemovePacket == null)
    {
      lock (_rpl)
      {
        if (m_RemovePacket == null)
        {
          m_RemovePacket = new RemoveItem(this);
          m_RemovePacket.SetStatic();
        }
      }
    }

    return m_RemovePacket;
  }
}

The analyzer warning relates to unsafe usage of the double checked locking pattern. As can be seen from the above code, double checked locking was applied to implement the singleton pattern. When attempting to get the Packet class instance and addressing the RemovePacket property, the getter checks the m_RemovePacket field for null. If the check is successful, we get into the body of the lock operator, where the field m_RemovePacket gets initialized. The plot thickens when the main thread has already initialized the m_RemovePacket variable through the constructor, but hasn't called the SetStatic() method yet.In theory, another thread can access the RemovePacket property at this very awkward moment. The check of m_RemovePacket for null will fail and the caller thread will get the reference to a half ready-to-use object. To solve this problem, we can create an intermediate variable of Packet class in the body of the lock operator, initialize the variable via the constructor and the SetStatic() method, and after assign it to the m_RemovePacket variable. This way, the body of the lock operator might look as follows:

分析仪警告与不安全使用双重检查的锁定方式有关。 从上面的代码可以看出，已应用双重检查锁定来实现单例模式。 尝试获取Packet类实例并解决RemovePacket属性时，getter会检查m_RemovePacket字段是否为null。 如果检查成功，我们将进入锁操作符的主体，其中将初始化字段m_RemovePacket 。 当主线程已通过构造函数初始化m_RemovePacket变量但尚未调用SetStatic()方法时，图变厚。理论上，另一个线程可以在此非常尴尬的时刻访问RemovePacket属性。 m_RemovePacket的null检查将失败，并且调用者线程将获得对半成品的引用。 为了解决这个问题，我们可以在锁运算符的主体中创建Packet类的中间变量，通过构造函数和SetStatic()方法初始化该变量，然后将其分配给m_RemovePacket变量。 这样，锁运算符的主体可能如下所示：

lock (_rpl)
{
  if (m_RemovePacket == null)
  {
    Packet instance = new RemoveItem(this);
    instance.SetStatic();
    m_RemovePacket = instance;
  }
}

It seems that the problem has been fixed and the code will work as expected. But not so fast.

看来问题已解决，代码可以按预期工作。 但是没有那么快。

Here's another thing: the analyzer offers to use the volatile keyword for a reason. In the release version of the program, the compiler might optimize and reorder calling lines of the SetStatic() method and assignment of the instance variable to the m_RemovePacket field (from the compiler's point of view, program semantics won't break). Here we get back to the point where we started — the m_RemovePacket variable might be uninitialized. We can't say exactly when this reordering may occur. We are even not sure if it happens at all, as the CLR version, the architecture of the used processor and other factors might affect it. It's still worth preventing this scenario. In this regard, one of the solutions (not the most productive) will be usage of the keyword volatile. The variable declared with the volatile modifier won't be object to displacements during compiler optimizations. The final code version might look as follows:

这是另一回事：分析器提供使用volatile关键字的原因。 在程序的发行版中，编译器可能会优化SetStatic()方法的调用行并对其进行重新排序，并将实例变量分配给m_RemovePacket字段(从编译器的角度来看，程序语义不会中断)。 在这里，我们回到开始的地方-m_RemovePacket变量可能未初始化。 我们无法确切说出何时可能发生这种重新排序。 我们甚至不确定它是否会发生，因为CLR版本，使用的处理器的体系结构和其他因素可能会影响它。 仍然值得防止这种情况。 在这方面，解决方案之一(不是最有效的)将是使用关键字volatile 。 用volatile修饰符声明的变量在编译器优化期间不会成为位移的对象。 最终的代码版本可能如下所示：

private volatile Packet m_RemovePacket;
....
private object _rpl = new object();
public Packet RemovePacket
{
  get
  {
    if (m_RemovePacket == null)
    {
      lock (_rpl)
      {
        if (m_RemovePacket == null)
        {
          Packet instance = new RemoveItem(this);
          instance.SetStatic();
          m_RemovePacket = instance;
        }
      }
    }

    return m_RemovePacket;
  }
}

In some cases, it's undesirable to use the volatile field due to some cost of accessing this field. Let's not dwell on this issue, noting simply that in this example, the atomic field writing is needed only once (when first accessing the property). However, volatile field declaration will lead to the fact that the compiler will atomically perform its each reading and writing, which might be non-optimal in terms of performance.

在某些情况下，由于访问该字段会产生一些开销，因此不希望使用volatile字段。 让我们不要再讨论这个问题，简单地指出，在此示例中，原子字段写入仅需要一次(首次访问该属性时)。 但是， 易失性字段声明将导致以下事实：编译器将自动执行其每次读写操作，这在性能方面可能不是最佳的。

Therefore, let's consider another way to avoid this analyzer warning. We can use the Lazy<T> type for backing m_RemovePacket field instead of double checked locking. As a result, we'll get rid of potential non-optimizations from declaring the volatile field. In this case, the body of the getter can be replaced by the initializing method, which will be passed to the constructor of the Lazy<T> instance:

因此，让我们考虑另一种避免此分析器警告的方法。 我们可以使用Lazy <T>类型来支持m_RemovePacket字段，而不是双重检查锁定。 结果，我们将通过声明volatile字段摆脱潜在的非优化。 在这种情况下，可以将getter的主体替换为初始化方法，该方法将传递给Lazy <T>实例的构造函数：

private Lazy<Packet> m_RemovePacket = new Lazy<Packet>(() =>
  {
    Packet instance = new RemoveItem(this);
    instance.SetStatic();
    return instance;
  }, LazyThreadSafetyMode.ExecutionAndPublication);

....
public Packet RemovePacket
{
  get
  {
    return m_RemovePacket.Value;
  }
}

The initializing method will be called only once when first accessing the instance of the Lazy type. In doing so, the Lazy<T> type will ensure thread security in case of a simultaneous multi-thread access to a property. The thread security mode is controlled by the second parameter of the Lazy constructor.

首次访问Lazy类型的实例时，初始化方法将仅被调用一次。 这样，在同时对属性进行多线程访问的情况下， Lazy <T>类型将确保线程安全。 线程安全模式由Lazy构造函数的第二个参数控制。

PVS-Studio警告： (PVS-Studio warning: )

V3131 The expression 'targeted' is checked for compatibility with the type 'IAxe', but is casted to the 'Item' type. HarvestTarget.cs 61

V3131已检查表达式“ targeted”与类型“ IAxe”的兼容性，但将其强制转换为“ Item”类型。 HarvestTarget.cs 61

protected override void OnTarget( Mobile from, object targeted )
{
  ....
  else if ( m_System is Lumberjacking &&
            targeted is IAxe && m_Tool is BaseAxe )
  {
    IAxe obj = (IAxe)targeted;
    Item item = (Item)targeted;
    ....
  }
  ....
}

The targeted variable was checked for the IAxe type, but not for the Item type, as reported by the analyzer.

分析器报告，检查了目标变量的IAxe类型，但没有检查项目类型。

PVS-Studio警告： (PVS-Studio warning: )

V3070 Uninitialized variable 'Zero' is used when initializing the 'm_LastMobile' variable. Serial.cs 29

V3070初始化'm_LastMobile'变量时使用未初始化的变量'Zero'。 Serial.cs 29

public struct Serial : IComparable, IComparable<Serial>
{
  private int m_Serial;

  private static Serial m_LastMobile = Zero;                // <=
  private static Serial m_LastItem = 0x40000000;

  public static Serial LastMobile { .... }
  public static Serial LastItem { .... }

  public static readonly Serial MinusOne = new Serial( -1 );
  public static readonly Serial Zero = new Serial( 0 );     // <=
  ....
  private Serial( int serial )
  {
    m_Serial = serial;
  }
  ....
}

Actually there is no error here, but writing in such a way is not the best practice. Due to m_LastMobile value assignment to Zero, the structure with the Serial() default constructor will be created, leading to m_Serial=0 initialization. Which is akin to calling new Serial(0). In fact, developers got lucky that serial is meant to be equal to 0. If another value had to be there, this would lead to an error.

实际上，这里没有错误，但是以这种方式编写不是最佳实践。 由于将m_LastMobile值分配为Zero ，将创建带有Serial()默认构造函数的结构，从而导致m_Serial = 0初始化。 类似于调用new Serial(0) 。 实际上，开发人员很幸运， 串行等于0 。 如果必须有另一个值，则将导致错误。

PVS-Studio警告： (PVS-Studio warning: )

V3063 A part of conditional expression is always true if it is evaluated: m_Serial <= 0x7FFFFFFF. Serial.cs 83

V3063如果条件表达式的一部分被求值，则始终为true：m_Serial <= 0x7FFFFFFF。 Serial.cs 83

public bool IsItem
{
  get
  {
    return ( m_Serial >= 0x40000000 && m_Serial <= 0x7FFFFFFF );
  }
}

0x7FFFFFFF is the maximum possible value that can contain 0x7FFFFFFF是可以包含 Int32. Therefore, whatever value the Int32的最大可能值。 因此，无论 m_Serial variable had, it would be less or equal to m_Serial变量具有什么值，它都将小于或等于 0x7FFFFFFF. 0x7FFFFFFF 。

PVS-Studio警告： (PVS-Studio warning: )

V3004 The 'then' statement is equivalent to the 'else' statement. Serialization.cs 1571

V3004'then'语句等效于'else'语句。 序列化.cs 1571

public override void WriteDeltaTime( DateTime value )
{
  ....
  try 
  { 
    d = new TimeSpan( ticks-now ); 
  }
  catch 
  {
    if( ticks < now ) 
      d = TimeSpan.MaxValue; 
    else 
      d = TimeSpan.MaxValue;
  }
  ....
}

The analyzer warns of a suspicious piece of code in which the true and false branches of the if operator fully match. Perhaps, TimeSpan.MinValue has to be in one of the branches.The same code was found in several other places:

分析器警告一段可疑的代码，其中if运算符的真假分支完全匹配。 也许TimeSpan.MinValue必须位于分支之一中，在其他几个地方也可以找到相同的代码：

V3004 The 'then' statement is equivalent to the 'else' statement. Item.cs 2103

V3004'then'语句等效于'else'语句。 Item.cs 2103

public virtual void Serialize( GenericWriter writer )
{
  ....
  
  if( ticks < now ) 
    d = TimeSpan.MaxValue; 
  else 
    d = TimeSpan.MaxValue;
  
  ....
}

V3004 The 'then' statement is equivalent to the 'else' statement. Serialization.cs 383

V3004'then'语句等效于'else'语句。 序列化.cs 383

public override void WriteDeltaTime( DateTime value )
{
  ....
  
  if( ticks < now ) 
    d = TimeSpan.MaxValue; 
  else 
    d = TimeSpan.MaxValue;
  
  ....
}

I used «the same code» expression for reason. It seems to me that copypaste is at play here as well, these two fragments look suspiciously alike.

我出于某种原因使用了“相同的代码”表达式。 在我看来，复制粘贴也在这里起作用，这两个片段看起来可疑地相似。

PVS-Studio警告： (PVS-Studio warning: )

V3051 An excessive type cast. The object is already of the 'Item' type. Mobile.cs 11237

V3051类型转换过多。 该对象已经是“项目”类型。 Mobile.cs 11237

public Item Talisman
{
  get
  {
    return FindItemOnLayer( Layer.Talisman ) as Item;
  }
}
public Item FindItemOnLayer( Layer layer )
{
  ....
}

This analyzer warning triggers when having redundant usage of the as operator. There is no error in this code fragment, but it also makes no sense to cast the object to its own type.

当冗余使用as运算符时，将触发此分析器警告。 此代码段中没有错误，但是将对象强制转换为自己的类型也没有意义。

PVS-Studio警告： (PVS-Studio warning: )

V3148 Casting potential 'null' value of 'toSet' to a value type can lead to NullReferenceException. Properties.cs 502

V3148将“ toSet”的潜在“空”值转换为值类型可能会导致NullReferenceException。 Properties.cs 502

public static string ConstructFromString( .... )
{
  object toSet;
  bool isSerial = IsSerial( type );

  if ( isSerial ) // mutate into int32
    type = m_NumericTypes[4];

  ....
  else if ( value == null )
  {
    toSet = null;
  }
  ....

  if ( isSerial ) // mutate back
    toSet = (Serial)((Int32)toSet);

  constructed = toSet;
  return null;
}

In this code section, let's pay attention to the scenario when the value variable is null. This way, null is assigned to the toSet variable. Further, if the variable isSerial == true, then toSet is cast to Int32, resulting in NRE.

在此代码部分中，让我们注意值变量为null的情况 。 这样， 会将 null分配给toSet变量。 此外，如果变量isSerial == true ，则toSet被强制转换为Int32 ，从而导致NRE 。

We can fix this code by adding 0 by default:

我们可以通过默认添加0来修复此代码：

toSet = (Serial)((Int32)(toSet ?? 0));

PVS-Studio警告： (PVS-Studio warning: )

V3031 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions 'pack == null' and 'pack != null'. BODBuyGump.cs 64

V3031过度检查可以简化。 “ ||” 运算符被相反的表达式'pack == null'和'pack！= null'包围。 BODBuyGump.cs 64

public override void OnResponse(Server.Network.NetState sender, RelayInfo info)
{
  ....
  if ( (pack == null) ||
       ((pack != null) &&
        (!pack.CheckHold(
                m_From,
                item,
                true,
                true,
                0,
                item.PileWeight + item.TotalWeight)) ) )
  {
    pv.SayTo(m_From, 503204);
    m_From.SendGump(new BOBGump(m_From, m_Book, m_Page, null));
  }
  ....
}

As the analyzer tells us, we can simplify this code:

正如分析器告诉我们的那样，我们可以简化以下代码：

if ((pack == null) || ((pack != null) && (!pack.CheckHold(....))))

On the left and right of the '||' operator, there are opposite expressions. Here the pack != null check is redundant, as before that the opposite condition is checked: pack == null, and these expressions are separated by the operator '||'. We can shorten this line as follows:

在“ ||”的左侧和右侧 运算符，有相反的表达式。 这里pack！= null检查是多余的，就像之前检查相反条件一样：pack == null ，这些表达式由运算符'||'分隔。 我们可以将这一行缩短如下：

if (pack == null || !pack.CheckHold(....))

PVS-Studio警告： (PVS-Studio warning: )

V3080 Possible null dereference. Consider inspecting 'winner'. CTF.cs 1302

V3080可能为空的取消引用。 考虑检查“优胜者”。 CTF.cs 1302

private void Finish_Callback()
{
  ....
  CTFTeamInfo winner = ( teams.Count > 0 ? teams[0] : null );

  .... 

  m_Context.Finish( m_Context.Participants[winner.TeamID] as Participant );
}

Suppose teams.Count is 0. Then winner = null. Further in the code, the winner.TeamID property is accessed without a check for null, leading to access by null reference.

假设team.Count为0。则获胜者=空。 在代码的进一步部分中，无需检查null即可访问winner.TeamID属性，从而导致通过null引用进行访问。

PVS-Studio警告： (PVS-Studio warning: )

V3041 The expression was implicitly cast from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. StormsEye.cs 87

V3041该表达式从'int'类型隐式转换为'double'类型。 考虑使用显式类型转换以避免丢失小数部分。 例如：double A =(double)(X)/ Y;。 StormsEye.cs 87

public static void Gain( Mobile from, Skill skill ) 
{
  ....
  if ( from.Player && 
     ( skills.Total / skills.Cap ) >= Utility.RandomDouble())
  ....
}

In this code fragment, the skills.Total variable is divided into skills.Cap (variables are of int type); the result is then implicitly converted into the double type, this is what the analyzer tells us about.

在该代码段中，skills.Total变量分为skills.Cap(变量是整数类型的); 然后将结果隐式转换为double类型，这就是分析器告诉我们的内容。

PVS-Studio警告： (PVS-Studio warning: )

V3085 The name of 'typeofObject' field in a nested type is ambiguous. The outer type contains static field with identical name. PropsGump.cs 744

V3085嵌套类型中的'typeofObject'字段名称不明确。 外部类型包含名称相同的静态字段。 PropsGump.cs 744

private static Type typeofObject = typeof( object );
....
private class GroupComparer : IComparer
{
  ....
  private static Type typeofObject = typeof( Object );
  ....
}

In the above code, the typeofObject variable was created in the inner class. Its problem is that there is a variable with the same name in the outer class, and that can cause errors. It is better not to allow this in order to reduce the probability of such errors due to inattention.

在上面的代码中， typeofObject变量是在内部类中创建的。 它的问题是，外部类中存在一个具有相同名称的变量，并且可能导致错误。 最好不要这样做，以减少由于疏忽引起的此类错误的可能性。

PVS-Studio警告： (PVS-Studio warning: )

V3140 Property accessors use different backing fields. WallBanner.cs 77

V3140属性访问器使用不同的后备字段。 WallBanner.cs 77

private bool m_IsRewardItem;

[CommandProperty( AccessLevel.GameMaster )]
public bool IsRewardItem
{
  get{ return m_IsRewardItem; }
  set{ m_IsRewardItem = value; InvalidateProperties(); }
}

private bool m_East;

[CommandProperty( AccessLevel.GameMaster )]
public bool East
{
  get{ return m_East; }
  set{ m_IsRewardItem = value; InvalidateProperties(); }
}

Here we can immediately notice an error that appeared due to copypaste. The set access method of the East property was supposed to assign the value for m_East, not for m_IsRewardItem.

在这里，我们可以立即注意到由于复制粘贴而出现的错误。 East属性的set访问方法应该为m_East分配值，而不是为m_IsRewardItem分配值。

PVS-Studio警告： (PVS-Studio warnings: )

V3012 The '?:' operator, regardless of its conditional expression, always returns one and the same value: 0xe7f. TreasureChestLevel2.cs 52

V3012不论条件表达式如何，“ ?:”运算符始终返回一个相同的值：0xe7f。 TreasureChestLevel2.cs 52

V3012 The '?:' operator, regardless of its conditional expression, always returns one and the same value: 0xe77. TreasureChestLevel2.cs 57

V3012'？：'运算符，无论其条件表达式如何，始终返回一个相同的值：0xe77。 TreasureChestLevel2.cs 57

private void SetChestAppearance()
{
  bool UseFirstItemId = Utility.RandomBool();

  switch( Utility.RandomList( 0, 1, 2, 3, 4, 5, 6, 7 ) )
  {
    ....
    case 6:// Keg
      this.ItemID = ( UseFirstItemId ? 0xe7f : 0xe7f );
      this.GumpID = 0x3e;
      break;

    case 7:// Barrel
      this.ItemID = ( UseFirstItemId ? 0xe77 : 0xe77 );
      this.GumpID = 0x3e;
      break;
  }
}

Here comes the illusion of choice :) Regardless of the UseFirstItemId value, this.ItemID will still be equal either to 0xe7f in the first case, or to 0xe77 — in the second.

这是一种选择的幻想：)不管UseFirstItemId值如何， this.ItemID要么在第一种情况下都等于0xe7f ，在第二种情况下都等于0xe77 。

PVS-Studio警告： (PVS-Studio warning: )

V3066 Possible incorrect order of arguments passed to 'OnSwing' method: 'defender' and 'attacker'. BaseWeapon.cs 1188

V3066传递给“ OnSwing”方法的参数的可能错误顺序：“防御者”和“攻击者”。 BaseWeapon.cs 1188

public virtual int AbsorbDamageAOS( Mobile attacker,
                                    Mobile defender,
                                    int damage )
{
  ....
  if ( weapon != null )
  {
    defender.FixedParticles(0x3779,
                            1,
                            15,
                            0x158B,
                            0x0,
                            0x3,
                            EffectLayer.Waist);
    weapon.OnSwing( defender, attacker );
  }
  ....
}

public virtual TimeSpan OnSwing( Mobile attacker, Mobile defender )
{
  return OnSwing( attacker, defender, 1.0 );
}

The analyzer found it suspicious that the OnSwing() method was passed arguments in reverse order. This may be the result of a bug.

分析器发现OnSwing()方法以相反的顺序传递参数令人怀疑。 这可能是错误的结果。

PVS-Studio警告： (PVS-Studio warning: )

V3092 Range intersections are possible within conditional expressions. Example: if (A > 0 && A < 5) {… } else if (A > 3 && A < 9) {… }. HouseFoundation.cs 1883

V3092在条件表达式中可能存在范围交点。 示例：if(A> 0 && A <5){…}否则if(A> 3 && A <9){…}。 HouseFoundation.cs 1883

public static bool IsFixture( int itemID )
{
  ....
  else if( itemID >= 0x319C && itemID < 0x31B0 ) 
    return true;
  // ML doors
  else if( itemID == 0x2D46 ||
           itemID == 0x2D48 ||
           itemID == 0x2FE2 ||
           itemID == 0x2FE4 )
    return true;
  else if( itemID >= 0x2D63 && itemID < 0x2D70 )
    return true;
  else if( itemID >= 0x319C && itemID < 0x31AF ) 
    return true;
  ....
}

The ranges checked in the conditions above intersect. This seemed suspicious to the analyzer. Even if this piece of code works correctly, it's still worth tweaking it. Let's imagine the situation where we need to rewrite the body of the last if so that the method will return false if the condition is true. If itemID equals, let's say, 0x319C, the method will return true anyway. This, in turn, will result in a waste of time searching for the bug.

在上述条件下检查的范围相交。 对于分析仪而言，这似乎令人怀疑。 即使这段代码可以正常工作，仍然值得对其进行调整。 让我们想象一下这种情况，我们需要重写了最后的身体，如果这样，如果条件为true，该方法将返回false。 如果itemID等于0x319C ，则该方法无论如何都将返回true 。 反过来，这将导致浪费时间来寻找错误。

结论 (Conclusion)

RunUO appeared quite long ago, a lot of work has been done. At the same time, using this project as an example we can fully realize the benefits of static analysis application on projects with a long history. The analyzer issued about 500 warnings for 543,000 lines of code (without the Low level), most of which didn't get into the article because of their similarity. You're welcome to check out the free license for open source projects to find out more about the analysis results.

RunUO出现很久以前，已经完成了很多工作。 同时，以该项目为例，我们可以充分认识到在历史悠久的项目上进行静态分析的好处。 分析器针对543,000行代码(不包含Low level )发出了大约500条警告，由于它们的相似性，大多数未进入本文。 欢迎您查看开源项目的免费许可证，以了解有关分析结果的更多信息。

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

pvs-stdio ue4