RunUO-Prüfung des PVS-Studio-Analysators

Bild 1

Dieser Artikel befasst sich mit der Überprüfung des RunUO-Projekts mit dem statischen Analysator PVS-Studio. RunUO ist ein Server-Software-Emulator für Ultima Online, ein Spiel, das einst die Herzen vieler MMORPG-Fans eroberte.

Einführung


RunUO ist ein Server-Software-Emulator für MMORPG Ultima Online. Das Ziel dieses Projekts ist es, eine stabile Software zu erstellen, die mit den offiziellen Servern von EA Games konkurrieren kann. RunUO wurde bereits im Jahr 2002 erstellt, verliert jedoch nicht an Relevanz und wird bis heute aktiv genutzt.

Der Zweck der Projektüberprüfung besteht darin, das Thema der statischen Analyse bekannt zu machen. Wir überprüfen verschiedene Projekte - Spiele ( Beispiel ), Bibliotheken ( Beispiel ), Messenger ( Beispiel ), Browser ( Beispiel ) und vieles mehr ( Beispiel , Beispiel , Beispiel)) um die Aufmerksamkeit eines vielfältigen Publikums auf sich zu ziehen. Mit diesen Artikeln möchten wir auf die Bedeutung der Verwendung statischer Analysen in der Entwicklung aufmerksam machen. Die statische Analyse macht den Code zuverlässiger und sicherer. Außerdem können Sie bei regelmäßiger Verwendung Fehler frühestens finden und beseitigen. Dies spart Entwicklern Zeit und Mühe, da niemand 50 Stunden damit verbringen möchte, nach dem Fehler zu suchen, den der Analysator finden kann.

Wir helfen auch der Open-Source-Community. Durch Artikel mit gefundenen Fehlern tragen wir zur Entwicklung von Open Source bei. In den Artikeln analysieren wir jedoch nicht alle Warnungen. Einige Warnungen schienen uns zu langweilig, um in den Artikel einzusteigen, andere erwiesen sich als falsch positiv usw. Daher sind wir bereit, eine kostenlose Lizenz für Open Source-Projekte bereitzustellen . Darüber hinaus mag das, was uns für den Artikel langweilig erschien, für die Entwickler des getesteten Projekts sehr interessant erscheinen, da die Entwickler des Projekts dennoch besser wissen, welche Probleme am kritischsten sind.

Codeteile, die bei der Prüfung des Analysatorberichts aufgefallen sind


PVS-Studio Warnung: V3010 Der Rückgabewert der Funktion 'Intern' muss verwendet werden. 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 );
}

Der Rückgabewert der Intern () -Methode wird nirgendwo berücksichtigt, wie vom Analysator angegeben. Vielleicht ist dies ein Fehler oder redundanter Code.

PVS-Studio Warnung: V3017 Es wurde ein Muster erkannt: (Element ist BasePotion) || ((Element ist BasePotion) && ...). Der Ausdruck ist zu groß oder enthält einen logischen Fehler. Cleanup.cs 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 )
  {
    ....
  }
}

Es gibt Unterausdrücke, die vereinfacht werden können. Ich werde sie aufschreiben, damit es klarer wird:
if (item is BasePotion || ( item is BasePotion && Core.ML ))

Angenommen, item ist BasePotion = to true , dann ist die Bedingung trotz Core.ML wahr . Wenn das Element BasePotion = bis false lautet , ist die Bedingung unabhängig vom Wert Core.ML erneut false . Ein solcher Code ist meistens einfach redundant. Es gibt jedoch schlimmere Situationen, in denen der Programmierer einen Fehler gemacht und die falsche Variable in den zweiten Unterausdruck geschrieben hat.

PVS-Studio Warnung: V3031 Eine übermäßige Überprüfung kann vereinfacht werden. Das '||' Der Operator ist von den entgegengesetzten Ausdrücken 'bPlayerOnly' und '! bPlayerOnly' umgeben. BaseCreature.cs 3005

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

Dieser Code ist entweder redundant oder fehlerhaft. Sein Problem ist, dass auf beiden Seiten von '||' sind im Sinne von Subexpression unterschiedlich. Wenn wir es so schneiden:

if ( m.Player || !bPlayerOnly )

dann wird sich nichts ändern.

PVS-Studio Warnung: V3001 Links und rechts vom '||' gibt es identische Unterausdrücke 'deed is SmallBrickHouseDeed'. Operator. 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 )
      ....
}

Ich denke, es lohnt sich nicht, hier etwas zu erklären, den nächsten oder falschen oder redundanten Code.

PVS-Studio Warnung: V3067 Es ist möglich, dass der Block 'else' vergessen oder auskommentiert wurde, wodurch die Betriebslogik des Programms geändert wurde. 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;

  ....
}

Eine seltene Warnung. Der Analysator hat die Codeformatierung nach der Anweisung #endregion als verdächtig eingestuft. Wenn Sie den Code nicht lesen, sieht er wie eine Zeile aus

i.Movable = !locked;

wird in jedem Fall ausgeführt, unabhängig von der Variablen i . Vielleicht wurden hier geschweifte Klammern vergessen ... Im Allgemeinen sollten Entwickler diesen Code überprüfen.

PVS-Studio Warnung: V3043 Die Betriebslogik des Codes entspricht nicht seiner Formatierung. Die Anweisung wird rechts eingerückt, aber immer ausgeführt. Möglicherweise fehlen geschweifte Klammern. Erdbeben.cs 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
  {
    ....
  }
}

In diesem Code fehlen möglicherweise geschweifte Klammern. Diese Schlussfolgerung kann aufgrund der seltsamen Formatierung des Codes im Hauptteil von if (! M.Player) gezogen werden .

PVS-Studio Warnung: V3083 Unsicherer Aufruf des Ereignisses 'ServerStarted', NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie es aufrufen. EventSink.cs 921

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

Diese Methode verwendet einen möglicherweise unsicheren Aufruf des RefreshStarted- Ereignishandlers , wie der Analysator angibt.

Warum ist es gefährlich? Stellen Sie sich diese Situation vor. Das ServerStarted- Ereignis hat nur einen Abonnenten. Im Moment zwischen der Überprüfung auf null und dem direkten Aufrufen des ServerStarted () -Ereignishandlers in einem anderen Thread wurde eine Abmeldung von diesem Ereignis vorgenommen. Dies löst eine NullReferenceException aus .

Der einfachste Weg, um diese Situation zu verhindern, besteht darin, einen sicheren Ereignisaufruf mit dem Operator '?' Bereitzustellen:

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

PVS-Studio Warnung: V3054 Möglicherweise unsichere, doppelt überprüfte Verriegelung. Verwenden Sie flüchtige Variablen oder Synchronisationsprimitive, um dies zu vermeiden. 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;
  }
}

Die Analysatorwarnung ist mit der unsicheren Verwendung des doppelt überprüften Sperrmusters verbunden. Wie Sie dem obigen Code entnehmen können, wurde eine doppelte Überprüfungssperre angewendet, um das generative Muster Loners zu implementieren. Wenn wir versuchen, eine Instanz der Packet- Klasse durch Zugriff auf die RemovePacket- Eigenschaft abzurufen , überprüft der Getter das Feld m_RemovePacket auf Gleichheit auf Null. Wenn die Prüfung bestanden ist, gelangen wir in den Hauptteil der lock- Anweisung , in der das Feld m_RemovePacket initialisiert wird . Eine interessante Situation tritt in dem Moment auf, in dem der Hauptthread die Variable m_RemovePacket bereits über den Konstruktor initialisiert, die Methode jedoch noch nicht aufgerufen hatSetStatic (). Theoretisch kann ein anderer Thread im sehr ungünstigen Moment auf die RemovePacket- Eigenschaft zugreifen . Das Überprüfen von m_RemovePacket auf Gleichheit mit Null schlägt nicht mehr fehl und der aufrufende Thread erhält einen Link zu einem unvollständig gebrauchsfertigen Objekt. Um dieses Problem zu lösen, können Sie eine Zwischenvariable der Packet- Klasse im Hauptteil der lock- Anweisung erstellen , sie über einen Konstruktoraufruf und die SetStatic () -Methode initialisieren und sie dann der Variablen m_RemovePacket zuweisen . In diesem Fall könnte der Hauptteil der lock- Anweisung folgendermaßen aussehen:

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

Das Problem scheint behoben zu sein und der Code funktioniert wie erwartet. Aber das ist nicht so.

Es gibt noch einen weiteren Punkt: Der Analysator schlägt nicht nur die Verwendung des Schlüsselworts volatile vor . In der Release - Version des Programms, die Compiler - Optimierung und Neuordnungs die Linien des Aufrufs zur durchführen kann setstatic () Methode und weisen Sie das Instanz - Variable auf das m_RemovePacket Feld (aus der Sicht des Compilers, nicht die Semantik des Programms verletzt werden). Und wir kehren wieder zu dem Punkt zurück, an dem wir begonnen haben - der Möglichkeit, die nicht initialisierte Variable m_RemovePacket zu erhalten. Es ist unmöglich genau zu sagen, wann diese Neuordnung stattfinden kann und ob sie überhaupt stattfinden wird: die CLR-Version, die Architektur des verwendeten Prozessors usw. Es lohnt sich immer noch, sich vor einem solchen Szenario zu schützen, und eine der Lösungen (die jedoch nicht die produktivste ist) besteht darin, das flüchtige Schlüsselwort zu verwenden . Eine Variable, die mit dem flüchtigen Modifikator deklariert wird, unterliegt während der Optimierung durch den Compiler keinen Permutationen. Ein endgültig festgelegter Code könnte folgendermaßen aussehen:

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

Manchmal kann die Verwendung eines flüchtigen Feldes aufgrund des Overheads beim Zugriff auf ein solches Feld unerwünscht sein. Wir werden nicht im Detail auf dieses Thema eingehen und lediglich darauf hinweisen, dass im betrachteten Beispiel ein Atomfelddatensatz nur einmal benötigt wird (wenn auf die Eigenschaft zum ersten Mal zugegriffen wird). Wenn Sie das Feld jedoch als flüchtig deklarieren, führt der Compiler das atomare Lesen und Schreiben durch das ist möglicherweise nicht optimal in Bezug auf die Leistung.

Daher besteht ein anderer Ansatz zur Korrektur dieser Analysatorwarnung, um zusätzlichen Overhead beim Deklarieren eines flüchtigen Felds zu vermeiden , darin, den Typ Lazy <T> zum Sichern des Felds m_RemovePacket zu verwendenstatt Doppelverriegelung zu überprüfen. In diesem Fall kann der Getter-Body durch die Initialisierungsmethode ersetzt werden, die an den Konstruktor der Lazy <T> -Instanz übergeben wird :

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

Die Initialisierungsmethode wird einmal aufgerufen, wenn zum ersten Mal auf den Lazy- Typ zugegriffen wird, und die Thread-Sicherheit beim gleichzeitigen Zugriff auf die Eigenschaft von mehreren Threads aus wird durch den Lazy <T> -Typ garantiert (der Thread-Sicherheitsmodus wird durch den zweiten Parameter des Lazy- Konstruktors gesteuert ).

PVS-Studio Warnung: V3131 Der Ausdruck "Ziel" wird auf Kompatibilität mit dem Typ "IAxe" überprüft, jedoch in den Typ "Element" umgewandelt. 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;
    ....
  }
  ....
}

Die gezielte Variable wurde geprüft , die Typ iAXE , aber niemand überprüft zu gehören , Artikel , wie die Analyseberichte.

PVS-Studio Warnung: V3070 Nicht initialisierte Variable 'Zero' wird beim Initialisieren der Variablen 'm_LastMobile' verwendet. 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;
  }
  ....
}

Insofern gibt es hier keinen Fehler, das Schreiben ist jedoch nicht sehr gut. Aufgrund der Zuweisung von m_LastMobile zum Wert einer nicht initialisierten Null wird mit dem Standardkonstruktor Serial () eine Struktur erstellt , die zur Initialisierung von m_Serial = 0 führt . Dies entspricht wiederum dem Aufruf einer neuen Seriennummer (0) . Tatsächlich haben die Entwickler das Glück, dass serial gleich 0 sein sollte. Wenn es einen anderen Wert gegeben hätte, würde dies zu einem Fehler führen.

PVS-Studio Warnung: V3063 Ein Teil des bedingten Ausdrucks ist immer wahr, wenn er ausgewertet wird: m_Serial <= 0x7FFFFFFF. Serial.cs 83

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

0x7FFFFFFF ist der maximal mögliche Wert, den Int32 enthalten kann . Unabhängig davon , welchen Wert die Variable m_Serial hat , ist sie in jedem Fall kleiner oder gleich 0x7FFFFFFF .

PVS-Studio Warnung: V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. Serialization.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;
  }
  ....
}

Der Analysator warnt vor einem verdächtigen Code, bei dem die wahren und falschen Zweige der if-Anweisung vollständig übereinstimmen. Vielleicht sollte einer der Zweige TimeSpan.MinValue haben. Der gleiche Code wurde an mehreren Stellen gefunden:

V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. Item.cs 2103

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

V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. Serialization.cs 383

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

Ich habe "Dies" aus einem Grund verwendet. Es ist sehr wahrscheinlich, dass es unter anderem nicht ohne Kopieren und Einfügen auskommt. Diese Codefragmente sehen sehr schmerzhaft aus.

Warnung PVS-Studio: V3051 Eine übermäßige Besetzung. Das Objekt ist bereits vom Typ 'Item'. Mobile.cs 11237

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

Diese Analysatorwarnung kann erhalten werden, wenn der as- Bediener übermäßig verwendet wird . In diesem Abschnitt des Codes gibt es keinen Fehler, aber es macht auch keinen Sinn, das Objekt in einen eigenen Typ umzuwandeln.

PVS-Studio- Warnung : V3148 Das Umwandeln des potenziellen 'Null'-Werts von' toSet 'in einen Werttyp kann zu NullReferenceException führen. Eigenschaften.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 diesem Abschnitt des Codes, sind wir in dem Fall interessiert , wenn der variable Wert ist null . Dann ist auch die Variable toSet null . Wenn die Variable Serial == true ist , wird toSet in Int32 umgewandelt , was zu NRE führt .

Sie können diesen Code beheben, indem Sie beispielsweise standardmäßig 0 hinzufügen:

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

PVS-Studio Warnung: V3031 Eine übermäßige Überprüfung kann vereinfacht werden. Das '||' Der Operator ist von entgegengesetzten Ausdrücken 'pack == null' und 'pack! = null' umgeben. 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));
  }
  ....
}

Dieser Code kann vereinfacht werden, wie der Analysator berichtet:

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

Links und rechts vom Operator '||' Es gibt Ausdrücke, deren Bedeutung entgegengesetzt ist. Hier ist das Prüfpaket! = Null redundant, da zuvor das entgegengesetzte Bedingungspaket == Null geprüft wird und diese Ausdrücke durch den Operator '||' getrennt werden. Diese Zeile kann wie folgt gekürzt werden:

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

PVS-Studio Warnung: V3080 Mögliche Null-Dereferenzierung. Betrachten Sie die Überprüfung der "Gewinner". 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 );
}

Nehmen wir an, Teams . Die Anzahl ist 0 . Dann ist Gewinner = null. Und weiter im Code wird auf die Eigenschaft won.TeamID zugegriffen, ohne nach null zu suchen , was zum Zugriff durch eine Nullreferenz führt.

PVS-Studio Warnung: V3041 Der Ausdruck wurde implizit vom Typ 'int' in den Typ 'double' umgewandelt. Erwägen Sie die Verwendung eines expliziten Typgusses, um den Verlust eines Bruchteils zu vermeiden. Ein Beispiel: 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())
  ....
}

Dieses Codefragment enthält die Operation zum Teilen der Variablen Skills.Total und Skills.Cap vom Typ int . Das Ergebnis wird dann implizit in den Typ double konvertiert , wie der Analysator berichtet.

PVS-Studio Warnung: V3085 Der Name des Felds 'typeofObject' in einem verschachtelten Typ ist nicht eindeutig. Der äußere Typ enthält ein statisches Feld mit identischem Namen. PropsGump.cs 744

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

Die Variable typeofObject wurde in diesem Codeabschnitt in einer verschachtelten Klasse erstellt . Das Problem ist, dass es in der äußeren Klasse eine Variable mit demselben Namen gibt und aufgrund dessen Fehler auftreten können. Es ist besser, dies nicht zuzulassen, um die Wahrscheinlichkeit solcher Fehler aufgrund von Unaufmerksamkeit zu verringern.

PVS-Studio Warnung: V3140 Property Accessors verwenden unterschiedliche Sicherungsfelder. 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(); }
}

Und hier ist ein Fehler aufgetreten, der durch Kopieren und Einfügen aufgetreten ist. Die festgelegte Zugriffsmethode der East- Eigenschaft bestand darin, einen Wert für m_East und nicht für m_IsRewardItem zuzuweisen .

PVS-Studio-

Warnungen : V3012 Der Operator '?:' Gibt unabhängig von seinem bedingten Ausdruck immer ein und denselben Wert zurück: 0xe7f. TreasureChestLevel2.cs 52

V3012 Der Operator '?:' Gibt unabhängig von seinem bedingten Ausdruck immer ein und denselben Wert zurück: 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;
  }
}

Irgendeine Art von Illusion der Wahl :) Unabhängig vom Wert von UseFirstItemId , this.ItemID wird entweder 0xe7f im ersten Fall, oder 0xe77 in den zweiten.

Warnung PVS-Studio: V3066 Möglicherweise falsche Reihenfolge der an die OnSwing-Methode übergebenen Argumente: 'Verteidiger' und 'Angreifer'. 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 );
}

Dem Analysator erschien es verdächtig, dass der OnSwing () -Methode Argumente in umgekehrter Reihenfolge übergeben wurden. Möglicherweise liegt dies an einem Fehler.

PVS-Studio Warnung: V3092 Bereichsüberschneidungen sind innerhalb von bedingten Ausdrücken möglich. Beispiel: if (A> 0 && A <5) {...} else 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;
  ....
}

Unter Bedingungen getestete Bereiche kreuzen sich. Dies schien dem Analysator verdächtig. Auch wenn dieser Code korrekt funktioniert, lohnt es sich, ihn zu reparieren. Stellen Sie sich eine Situation , in der wir den Körper der letzten neu zu schreiben benötigt , wenn so , dass, wenn die Bedingung erfüllt ist, kehrt das Verfahren falsch . Wenn itemID ist gleich, sagen wir, 0x319C wird das Verfahren immer noch zurückkehren wahr . Dies führt wiederum zu einem Zeitverlust bei der Suche nach Fehlern.

Fazit


RunUO wurde vor langer Zeit erstellt, es wurde viel Arbeit geleistet. Am Beispiel dieses Projekts können Sie jedoch die Vorteile der statischen Analyse für Projekte mit einem Verlauf erkennen. Der Analysator für 543.000 Zeilen Projektcode gab ungefähr 500 Warnungen aus (ohne den niedrigen Pegel ), von denen die meisten aufgrund ihrer Einheitlichkeit nicht im Artikel enthalten waren. Um das Ergebnis der Analyse genauer kennenzulernen, empfehle ich die Verwendung einer kostenlosen Lizenz für Open Source-Projekte .



Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Ekaterina Nikiforova. RunUO Check mit dem PVS-Studio Analyzer .

Source: https://habr.com/ru/post/undefined/


All Articles