🚵🏽 🎶 ❗️ PVS-Studio分析仪RunUO检查 💳 🌗 🧒

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

介绍

RunUO是MMORPG Ultima Online的服务器软件模拟器。该项目的目标是创建稳定的软件，使其能够与EA Games的官方服务器竞争。 RunUO早在2002年就创建了，但是并没有失去相关性，并且一直活跃到今天。

项目验证的目的是普及静态分析的主题。我们检查各种项目-游戏（示例），库（示例），Messenger（示例），浏览器（示例）以及更多内容（示例，示例，示例）以吸引多元化的受众。通过这些文章，我们试图吸引人们注意在开发中使用静态分析的重要性。静态分析使代码更可靠，更安全。同样，通过常规使用，您可以尽早发现并消除错误。这节省了开发人员的时间和精力，因为没有人愿意花费50个小时来寻找分析仪可以找到的错误。

我们还帮助开源社区。通过发现错误的文章，我们为开源的发展做出了贡献。但是，在本文中，我们不会分析所有警告。有些警告似乎太无聊，以至于我们无法进入本文，有些警告被证明是误报，等等。因此，我们准备为开源项目提供免费许可证。而且，对于我们来说，让本文感到无聊的内容对于正在测试的项目的开发人员来说似乎很有趣，因为该项目的开发人员仍然更加了解哪些问题最为关键。

检查分析器报告时引起注意的代码段

PVS-Studio警告： 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 );
}

分析器指出，任何地方都不会考虑Intern（）方法的返回值。也许这是一个错误或冗余代码。

PVS-Studio警告： 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 )
  {
    ....
  }
}

有可以简化的子表达式。我将它们写出来以便更清楚：

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

假设，产品BasePotion =为真，则条件为真，尽管Core.ML。如果该项为BasePotion =，则条件为false，无论值Core.ML为何。这样的代码通常只是简单的冗余，但是，当程序员犯了一个错误并在第二个子表达式中写入了错误的变量时，情况更糟。

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

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

此代码是冗余的或错误的。他的问题是“ ||”的两边在子表达的意义上是不同的。如果我们这样切：

if ( m.Player || !bPlayerOnly )

然后什么都不会改变。

PVS-Studio警告： V3001在'||'的左侧和右侧有相同的子表达式'deed is SmallBrickHouseDeed'。操作员。房地产经纪人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 )
      ....
}

我认为没有必要在此解释任何内容，包括下一个或错误的代码或多余的代码。

PVS-Studio警告： 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;

  ....
}

非常罕见的警告。分析人员发现#endregion指令后的代码格式可疑。如果您不阅读代码，它看起来像一行

i.Movable = !locked;

无论变量i是什么，都将在任何情况下执行。也许这里花括号被遗忘了……通常，开发人员应检查此代码。

PVS-Studio警告： 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
  {
    ....
  }
}

该代码中可能缺少花括号。可以由于if（！M.Player）主体中代码的奇怪格式而得出此结论。

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

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

正如分析器指出的那样，此方法使用对RefreshStarted事件处理程序的潜在不安全调用。

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

防止这种情况的最简单方法是使用'？。'运算符提供安全的事件调用：

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

PVS-Studio警告： 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;
  }
}

分析仪警告与不安全使用双重检查的锁定方式有关。从上面的代码中可以看到，应用了双重检查锁来实现生成模式-Loners。当我们尝试通过访问RemovePacket属性获取Packet类的实例时，getter会检查m_RemovePacket字段的相等性是否为零。如果检查通过，那么我们进入lock语句的主体，其中m_RemovePacket字段已初始化。当主线程已通过构造函数初始化m_RemovePacket变量但尚未调用方法时，会出现一种有趣的情况。SetStatic（）。从理论上讲，另一个线程可以在非常不方便的时刻访问RemovePacket属性。检查m_RemovePacket的相等性是否为零将不再失败，并且调用线程将收到指向未完全使用的对象的链接。为了解决此问题，您可以在lock语句的主体中创建Packet类的中间变量，通过构造函数调用和SetStatic（）方法对其进行初始化，然后将其分配给m_RemovePacket变量。在这种情况下，lock语句的主体可能如下所示：

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

该问题似乎已解决，该代码将按预期工作。但是事实并非如此。

还有一点：分析器不仅仅建议使用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;
  }
}

有时由于访问此类字段的开销而可能不希望使用易失性字段。我们不会详细讨论该主题，只是指出在所考虑的示例中，原子字段记录仅需要一次（第一次访问该属性时），但是，将字段声明为volatile将导致编译器进行原子读写就性能而言可能不是最佳的。

因此，另一种纠正此分析器警告以避免声明可变字段的额外开销的方法是使用Lazy <T>类型来支持m_RemovePacket字段而不是仔细检查锁定。在这种情况下，可以将getter主体替换为initializer方法，该方法将传递给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;
  }
}

首次访问Lazy类型时，将调用一次初始化方法，并且通过Lazy <T>类型可以保证在同时从多个线程访问该属性的情况下的流安全性（线程安全模式由Lazy构造函数的第二个参数控制）。

PVS-Studio警告： 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;
    ....
  }
  ....
}

正如分析器报告的那样，检查了目标变量是否属于IAxe类型，但没有检查任何人属于Item。

PVS-Studio警告：初始化“ m_LastMobile”变量时，使用V3070未初始化的变量“零”。 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;
  }
  ....
}

因此，这里没有错误，但是编写它不是很好。由于将m_LastMobile分配给未初始化的零的值，将创建具有默认构造函数Serial（）的结构，这将导致m_Serial = 0的初始化。而这等效于调用新的Serial（0）。实际上，开发人员很幸运，serial应该等于0，如果应该有任何其他值，这将导致错误。

PVS-Studio警告： V3063如果对条件表达式进行评估，则该条件表达式的一部分始终为true：m_Serial <= 0x7FFFFFFF。 Serial.cs 83

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

0x7FFFFFFF是Int32可以包含的最大可能值。因此，无论m_Serial变量具有什么值，它在任何情况下都将小于或等于0x7FFFFFFF。

PVS-Studio警告： 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;
  }
  ....
}

分析器警告一段可疑的代码，其中if语句的真假分支完全重合。也许其中一个分支应具有TimeSpan.MinValue。在以下几个地方发现了相同的代码：

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

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

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

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

我之所以使用“ This”是有原因的，除其他因素外，很有可能没有复制粘贴，这些代码片段看起来非常痛苦。

警告PVS-Studio： V3051类型转换过多。该对象已经是“项目”类型。Mobile.cs 11237

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

如果过多使用as运算符，则可以得到此分析器警告。代码的这一部分没有错误，但是将对象转换为自己的类型也没有意义。

PVS-Studio警告： 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;
}

在这部分代码中，我们感兴趣的情况下，当变量的值是零。然后，toSet变量也为null。此外，如果变量isSerial == true，则将toSet强制转换为Int32，这将导致NRE。

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

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

PVS-Studio警告： 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));
  }
  ....
}

由于分析仪报告，此代码可以简化：

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

在运算符“ ||”的左侧和右侧在表达的含义上是相反的。这里的check pack！= Null是多余的，因为在此之前检查了相反的条件pack == null，并且这些表达式由运算符'||'分隔。该行可以缩短，如下所示：

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

PVS-Studio警告： 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 );
}

可以说teams.Count为0。然后获胜者=空。进一步在代码中，无需检查null 即可访问winner.TeamID属性，这将导致通过null引用进行访问。PVS-Studio警告： 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())
  ....
}

此代码片段包含将所述变量的操作skills.Total和skills.Cap，它们是类型的INT，并且将结果然后隐式转换为类型双，作为分析仪报告。

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

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

变量typeofObject是在此部分代码中的嵌套类中创建的。它的问题是，在外部类中有一个具有相同名称的变量，因此会发生错误。最好不要这样做，以减少由于疏忽引起的此类错误的可能性。

PVS-Studio警告： 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(); }
}

由于复制粘贴，这里出现了一个错误。East属性的设置访问方法是为m_East分配一个值，而不是m_IsRewardItem。PVS-Studio 警告： V3012不论条件表达式如何，“ ?:”运算符始终返回一个相同的值：0xe7f。 TreasureChestLevel2.cs 52 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;
  }
}

某种选择，无论其价值的:)错觉UseFirstItemId，this.ItemID将是要么0xe7f在第一种情况下，或0xe77在第二位。

警告PVS-Studio： 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 );
}

对于分析器来说，似乎似乎怀疑OnSwing（）方法是以相反的顺序传递参数的。也许这是由于错误。

PVS-Studio警告： 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;
  ....
}

在条件下测试的范围相交。对于分析仪而言，这似乎令人怀疑。即使这段代码可以正常工作，仍然值得修复。试想一下，在这里我们需要重写了最后的身体状况，如果这样，如果条件为真，该方法返回假。如果itemID等于0x319C，则该方法仍将返回true。反过来，这将浪费寻找错误的时间。

结论

RunUO创建于很久以前，已经完成了很多工作，但是，使用该项目的示例，您可以看到对具有历史记录的项目使用静态分析的好处。用于54.3万行项目代码的分析器发出了大约500条警告（不包括“低” 级别），由于它们的统一性，大多数未出现在文章中。为了更详细地了解分析结果，我建议对开源项目使用免费许可证。

如果您想与说英语的读者分享这篇文章，请使用以下链接：Ekaterina Nikiforova。通过PVS-Studio分析仪进行RunUO检查。

PVS-Studio分析仪RunUO检查

介绍

检查分析器报告时引起注意的代码段

结论

More articles: