Geekly articles weekly

Emby Source Code Analyse mit dem PVS-Studio Analyzer

Bild 3

Emby ist neben Plex und Kodi ein ziemlich bekannter Medienserver. In diesem Artikel werden wir versuchen, den Quellcode mithilfe des statischen Analysators PVS-Studio zu überprüfen. Auf der offiziellen Emby-Website befindet sich eine kleine Marke „Built with ReSharper“. Nun, es ist noch interessanter.

PVS-Studio Code Analyzer


PVS-Studio läuft auf 64-Bit-Systemen unter Windows, Linux und MacOS. Er weiß, wie man nach Fehlern im Quellcode von Programmen sucht, die in C, C ++, C # und Java geschrieben sind.

Geprüftes Projekt


Emby ist ein Medienserver, dessen Quellcode auf GitHub zu finden ist. Sie können damit Sendungen organisieren und von jedem Gerät aus auf Medieninhalte (Video, Audio, Fotos) zugreifen. Drücken Sie von der offiziellen Emby- Website:

  • Emby konvertiert Ihre Medien automatisch im Handumdrehen für die Wiedergabe auf jedem Gerät.
  • Umfangreiche Kindersicherungsfunktionen erleichtern die Kontrolle des Zugriffs auf Inhalte, die für Familien mit kleinen Kindern relevant sind.
  • Emby organisiert Ihre Medien in einfachen und eleganten Präsentationen. Sie werden niemals gleich aussehen.
  • Streaming von Daten mit Synchronisation in der Cloud;
  • Teilen Sie Ihre Mediendateien ganz einfach mit Freunden und der Familie.
  • Und vieles mehr.

Codeteile, die bei der Prüfung eines Analyseberichts aufgefallen sind


PVS-Studio Warnung: V3001 Es gibt identische Unterausdrücke 'c! =' <'' Links und rechts vom Operator '&&'. HttpListenerRequest.Managed.cs 49

internal void SetRequestLine(string req) { .... if ((ic >= 'A' && ic <= 'Z') || (ic > 32 && c < 127 && c != '(' && c != ')' && c != '<' && // <= c != '<' && c != '>' && c != '@' && c != ',' && c != ';' && // <= c != ':' && c != '\\' && c != '"' && c != '/' && c != '[' && c != ']' && c != '?' && c != '=' && c != '{' && c != '}')) continue; .... }

Der Analysator hat einen doppelten Unterausdruck c! = '<' Festgestellt. Möglicherweise hat sich ein Fehler in diesen Code eingeschlichen, und anstelle von '<' hätte etwas anderes verwendet werden müssen. Oder, wahrscheinlicher, der zweite Unterausdruck ist überflüssig und Sie müssen ihn nur löschen.

PVS-Studio Warnung: V3001 Es gibt identische Unterausdrücke 'SmbConstants.AttrHidden' links und rechts vom '|' Betreiber. SmbComDelete.cs 29

 internal SmbComDelete(string fileName) { Path = fileName; Command = SmbComDelete; _searchAttributes = SmbConstants.AttrHidden | SmbConstants.AttrHidden | SmbConstants.AttrSystem; }

Der Analysator hat erneut einen Tippfehler festgestellt, der mit wiederholten Unterausdrücken verbunden ist. Mit Blick auf die Zukunft werde ich sagen, dass es in diesem Projekt ein bisschen zu viele ähnliche Fehler gibt, die durch Nachlässigkeit verursacht wurden. Ich beschuldige die Schöpfer von Emby überhaupt nicht, wir sind manchmal auf diese Weise verstreut ( Beispiele , Beispiele , Beispiele ), und um vor solchen Fehlern sicher zu sein, gibt es nur eine statische Analyse.

PVS-Studio Warnung: V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. SqliteItemRepository.cs 5648

 private QueryResult<Tuple<BaseItem, ItemCounts>> GetItemValues(....) { .... if (typesToCount.Length == 0) { whereText += " And CleanName In (Select CleanValue from ItemValues where " + typeClause + " AND ItemId in (select guid from TypedBaseItems" + innerWhereText + "))"; } else { //whereText += " And itemTypes not null"; whereText += " And CleanName In (Select CleanValue from ItemValues where " + typeClause + " AND ItemId in (select guid from TypedBaseItems" + innerWhereText + "))"; } .... }

Und das ist sehr ähnlich zu dem Problem, das beim Kopieren und Einfügen auftrat, weil ob und sonst Blockkörper völlig identisch sind. Warum mussten Sie die Größen des Arrays typesToCount überprüfen, wenn unabhängig von der Anzahl der darin geschriebenen Elemente derselbe Code ausgeführt wird - dies ist nur Entwicklern bekannt.

PVS-Studio Warnung: V3005 Die Variable '_validProviderIds' ist sich selbst zugewiesen. BaseNfoParser.cs 77

 private Dictionary<string, string> _validProviderIds; .... public void Fetch(....) { .... _validProviderIds = _validProviderIds = new Dictionary<....>(....); .... }

Wieder ein Tippfehler, der die Zuweisung einer Variablen an sich selbst zur Folge hatte. Es lohnt sich, diesen Codeabschnitt zu überprüfen.

PVS-Studio Warnung: V3008 Der Variablen 'Chapters' werden nacheinander zweimal Werte zugewiesen. Vielleicht ist das ein Fehler. Check lines: 29, 28. Title.cs 29

 public Title(uint titleNum) { ProgramChains = new List<ProgramChain>(); Chapters = new List<Chapter>(); Chapters = new List<Chapter>(); TitleNumber = titleNum; }

Wieder Unaufmerksamkeit und Tippfehler ... Der Variablen Chapters wird zweimal ein Wert zugewiesen. Es ist klar, dass dieser zusätzliche Code nichts Schlimmes bedroht, aber dennoch. Es macht keinen Sinn, in diesem Beispiel zu verweilen, lasst uns weitermachen.

PVS-Studio Warnung: V3013 Es ist merkwürdig, dass der Hauptteil der ' Lesen' -Funktion dem Hauptteil der' Schreiben'-Funktion (407, Zeile 415) vollständig entspricht. BaseSqliteRepository.cs 407

 public static IDisposable Read(this ReaderWriterLockSlim obj) { return new WriteLockToken(obj); } public static IDisposable Write(this ReaderWriterLockSlim obj) { return new WriteLockToken(obj); } private sealed class WriteLockToken : IDisposable { private ReaderWriterLockSlim _sync; public WriteLockToken(ReaderWriterLockSlim sync) { _sync = sync; sync.EnterWriteLock(); } public void Dispose() { if (_sync != null) { _sync.ExitWriteLock(); _sync = null; } } }

Die Körper der Lese- und Schreibfunktionen sind absolut identisch, wie im Warnungstext des Analysegeräts angegeben. Die EnterWriteLock- Methode wird benötigt, um die Sperre im Schreibmodus aufzurufen . Für den Lesemodus empfiehlt sich die Verwendung der EnterReadLock- Methode, mit der eine Ressource in mehreren Threads gleichzeitig gelesen werden kann.

Der Entwickler sollte sich diesen Abschnitt des Codes genauer ansehen, da möglicherweise ein Fehler vorliegt. Darüber hinaus wurde im Code die folgende Klasse gefunden, die nicht verwendet wird:

 private sealed class ReadLockToken : IDisposable { private ReaderWriterLockSlim _sync; public ReadLockToken(ReaderWriterLockSlim sync) { _sync = sync; sync.EnterReadLock(); } public void Dispose() { if (_sync != null) { _sync.ExitReadLock(); _sync = null; } } }

PVS-Studio Warnung: V3021 Es gibt zwei If-Anweisungen mit identischen Bedingungsausdrücken. Die erste 'if'-Anweisung enthält die Methode return. Dies bedeutet, dass die zweite "if" -Anweisung sinnlos ist. SkiaEncoder.cs 537

 public string EncodeImage(string inputPath, DateTime dateModified, string outputPath, bool autoOrient, ImageOrientation? orientation, int quality, ImageProcessingOptions options, ImageFormat selectedOutputFormat) { if (string.IsNullOrWhiteSpace(inputPath)) { throw new ArgumentNullException("inputPath"); } if (string.IsNullOrWhiteSpace(inputPath)) { throw new ArgumentNullException("outputPath"); } .... }

Hier hat der Entwickler anscheinend die ersten vier Zeilen kopiert, aber vergessen, die getestete Variable von inputPath in outputPath zu ändern . Der Code enthält mehrere Zeilen, die outputPath verwenden, ohne auf null zu prüfen, was dazu führen kann, dass eine Ausnahme ausgelöst wird.

PVS-Studio Warnungen:

  • V3022 Der Ausdruck 'processUnsupportedFrame (frame, CloseStatusCode.PolicyViolation, null)' ist immer falsch. WebSocket.cs 462
  • V3022 Der Ausdruck 'processCloseFrame (frame)' ist immer falsch. WebSocket.cs 461
  • V3022 Ausdruck 'frame.IsClose? processCloseFrame (frame): processUnsupportedFrame (frame, CloseStatusCode.PolicyViolation, null) 'ist immer false. WebSocket.cs 460
  • V3022 Der Ausdruck 'processPongFrame (frame)' ist immer wahr. WebSocket.cs 459
  • V3022 Der Ausdruck 'processPingFrame (frame)' ist immer wahr. WebSocket.cs 457
  • V3022 Der Ausdruck 'processDataFrame (frame)' ist immer wahr. WebSocket.cs 455
  • V3022 Ausdruck ist immer falsch. WebSocket.cs 448

 private bool processWebSocketFrame(WebSocketFrame frame) { return frame.IsCompressed && _compression == CompressionMethod.None ? processUnsupportedFrame(....) : frame.IsFragmented ? processFragmentedFrame(frame) : frame.IsData ? processDataFrame(frame) : frame.IsPing ? processPingFrame(frame) : frame.IsPong ? processPongFrame(frame) : frame.IsClose ? processCloseFrame(frame) : processUnsupportedFrame(....); } private bool processUnsupportedFrame(....) { processException(....); return false; } private bool processDataFrame(WebSocketFrame frame) { .... return true; } private bool processPingFrame(WebSocketFrame frame) { var mask = Mask.Unmask; return true; } private bool processPongFrame(WebSocketFrame frame) { _receivePong.Set(); return true; } private bool processCloseFrame(WebSocketFrame frame) { var payload = frame.PayloadData; close(payload, !payload.ContainsReservedCloseStatusCode, false); return false; }

In meinem Konto gibt es im Vergleich zum Rest des PVS-Studio-Teams nicht so viele, die vom Projektanalysator überprüft wurden. Dies hängt möglicherweise damit zusammen, dass ich zum ersten Mal sehe, dass in 13 Codezeilen bis zu 13 Warnungen des Analysators ausgegeben werden (etwas mehr als jede zweite Zeile). Daher glaube ich, dass dieser Code es verdient, in den Artikel aufgenommen zu werden. Als nächstes schreibe ich auf, worauf der Analysator speziell schwört.

  1. Zunächst wird der Ausdruck frame.IsCompressed && _compression == CompressionMethod.None berücksichtigt . Wenn der Ausdruck true ist, wird die processUnsupportedFrame- Methode ausgeführt, die in jedem Fall false zurückgibt (daher die erste Warnung des Analysators). Wenn es falsch war, fahren Sie mit dem nächsten Punkt fort.
  2. Als nächstes wird der Wert von frame.IsFragmented überprüft . Es gibt keine Probleme, also lasst uns weitermachen.
  3. Hier wird der Wert von frame.IsData geprüft . Wenn true, gibt die processDataFrame- Methode trotzdem true zurück . Daher die zweite Warnung des Analysators.
  4. Dann überprüfen wir frame.IsPing . True - processPingFrame gibt true zurück . Dies ist die dritte Warnung des Analysators.
  5. Wir schauen uns frame.IsPong an . Alles ist ähnlich wie beim Vorgänger.
  6. Und der letzte: frame.IsClose . processCloseFrame und processUnsupportedFrame geben auf jeden Fall false zurück .

Bild 1

Ich hoffe, ich habe dich nicht langweilen. Es wird weiter einfacher sein.

PVS-Studio Warnung: V3085 Der Name des Feldes 'RtpHeaderBytes' in einem verschachtelten Typ ist nicht eindeutig. Der äußere Typ enthält ein statisches Feld mit identischem Namen. HdHomerunUdpStream.cs 200

 public class HdHomerunUdpStream : LiveStream, IDirectStreamProvider { .... private static int RtpHeaderBytes = 12; public class UdpClientStream : Stream { private static int RtpHeaderBytes = 12; private static int PacketSize = 1316; private readonly MediaBrowser.Model.Net.ISocket _udpClient; bool disposed; .... } .... }

Die verschachtelte Klasse UdpClientStream verwendet ein Feld mit einem Namen, der dem in der externen Klasse HdHomerunUdpStream verwendeten ähnlich ist . Dies ist kein Fehler, aber es ist ein guter Grund zu prüfen, ob mit diesem Code alles in Ordnung ist. Das Problem besteht darin, dass diese beiden Variablen möglicherweise verwechselt werden und der Code nicht wie vom Entwickler beabsichtigt funktioniert, obwohl dies keine Beschwerden des Compilers zur Folge hat.

PVS-Studio Warnungen:

  • V3090 Unsichere Verriegelung eines Typs. Alle Instanzen eines Typs haben dasselbe 'Type'-Objekt. Lmhosts.cs 49
  • V3090 Unsichere Verriegelung eines Typs. Alle Instanzen eines Typs haben dasselbe 'Type'-Objekt. Lmhosts.cs 57

 public class Lmhosts { public static NbtAddress GetByName(string host) { lock (typeof(Lmhosts)) { return GetByName(new Name(host, 0x20, null)); } } internal static NbtAddress GetByName(Name name) { lock (typeof(Lmhosts)) { .... } } }

Der Analysator warnt, dass in diesem Codefragment eine unsichere Sperre verwendet wird. Die Verwendung der Sperre ist unerwünscht, da das zum Sperren verwendete Objekt gemeinsam genutzt wird und an anderer Stelle gesperrt werden kann, ohne dass der Entwickler, der dieses Objekt zum ersten Mal verwendet hat, davon Kenntnis hat. Das Ergebnis ist ein Deadlock für dasselbe Objekt.

Idealerweise sollten Sie ein privates Feld zum Sperren verwenden, zum Beispiel so:

 private Object locker = new Object(); public static NbtAddress GetByName(string host) { lock (locker) { return GetByName(new Name(host, 0x20, null)); } }

PVS-Studio Warnung: V3142 Unerreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. HdHomerunHost.cs 621

 protected override async Task<ILiveStream> GetChannelStream(....) { .... var enableHttpStream = true; if (enableHttpStream) { mediaSource.Protocol = MediaProtocol.Http; var httpUrl = channelInfo.Path; // If raw was used, the tuner doesn't support params if (!string.IsNullOrWhiteSpace(profile) && !string.Equals(profile, "native", StringComparison.OrdinalIgnoreCase)) { httpUrl += "?transcode=" + profile; } mediaSource.Path = httpUrl; return new SharedHttpStream(....); } return new HdHomerunUdpStream(....); }

Der Analyzer gibt an, dass die letzte Zeile in diesem Codefragment niemals ausgeführt wird. Und warum deklariert und setzt der Entwickler die Variable enableHttpStream auf true und überprüft diese Variable dann sofort in der if-Anweisung?

Vielleicht ist dieser Code einfach überflüssig. In jedem Fall lohnt sich eine Überprüfung.

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

 public void OnRefreshStart(BaseItem item) { .... if (RefreshStarted != null) { RefreshStarted(this, new GenericEventArgs<BaseItem>(item)); } }

Diese Methode verwendet einen möglicherweise unsicheren Aufruf des RefreshStarted- Ereignishandlers, wie vom Analyzer angegeben.

Jetzt werde ich erklären, warum der Analysator diesen Aufruf RefreshStarted im Code als unsicher eingestuft hat. Angenommen, zwischen der Überprüfung auf Null und dem direkten Aufrufen des Ereignishandlers im if- Body wird eine Abmeldung vom Ereignis in einem anderen Thread durchgeführt. Und wenn keine weiteren Abonnenten vorhanden sind, ist RefreshStarted null . In einem anderen Thread, in dem bereits eine Nullprüfung stattgefunden hat, wird die folgende Zeile ausgeführt:
 RefreshStarted(this, new GenericEventArgs<BaseItem>(item));

Dies löst eine NullReferenceException aus .

PVS-Studio Warnung: V3029 Die bedingten Ausdrücke der nebeneinander stehenden ' if' -Anweisungen sind identisch. Überprüfen Sie die Zeilen: 142, 152. LocalImageProvider.cs 142

 private void PopulateImages(....) { .... // Logo if (!isEpisode && !isSong && !isPerson) { added = AddImage(....); if (!added) { added = AddImage(....); } } // Art if (!isEpisode && !isSong && !isPerson) { AddImage(....); } .... }

In diesem Codeteil stehen zwei if-Anweisungen mit denselben Bedingungen nebeneinander. Die Körper der if-Anweisungen sind unterschiedlich. Schwer zu sagen, ob dies ein Fehler oder ein redundanter Code ist. Vielleicht ist alles in Ordnung und der Autor wollte nur zwei verschiedene Aktionen logisch trennen, von denen eine mit einem bestimmten „Logo“ und die andere mit einer bestimmten „Kunst“ verknüpft ist.

PVS-Studio Warnung: V3041 Der Ausdruck wurde implizit vom Typ 'int' in den Typ 'double' umgewandelt. Erwägen Sie die Verwendung einer expliziten Typumwandlung, um den Verlust eines Bruchteils zu vermeiden. Ein Beispiel: double A = (double) (X) / Y; LiveTvManager.cs 1085

 private async Task RefreshChannelsInternal(....) { .... double progressPerService = _services.Length == 0 ? 0 : 1 / _services.Length; .... }

Dieser Code enthält die Operation zum Teilen ganzzahliger Datentypen. Der resultierende Wert wird implizit in einen Gleitkommatyp konvertiert, was äußerst verdächtig ist.

Tatsächlich ist die Variable progressPerService nur dann gleich 1.0, wenn _services.Length = 1 ist . Für alle anderen _services.Length- Werte ist das Ergebnis 0.0 .

Anscheinend sollte es hier geschrieben werden:

 double progressPerService = _services.Length == 0 ? 0 : (double)1 / _services.Length;


PVS-Studio Warnung: V3050 Möglicherweise falsches HTML. Das </ u> schließende Tag wurde gefunden, während das </ i> -Tag erwartet wurde. SrtParserTests.cs 64

 public void TestParse() { var expectedSubs = new SubtitleTrackInfo { TrackEvents = new SubtitleTrackEvent[] { .... new SubtitleTrackEvent { Id = "6", StartPositionTicks = 330000000, EndPositionTicks = 339990000, Text = "This contains nested <b>bold, <i>italic, <u>underline</u> and <s>strike-through</s></u></i></b> HTML tags" }, .... } }; }

Achten Sie auf die Linie

 <u>underline</u>

Es hat bereits ein schließendes Tag </ u> . Als nächstes sehen wir diesen Text:
 </s></u></i></b> HTML tags"

Hier ist das schließende Tag </ u> überflüssig, wie der Analysator angibt.
Warnung PVS-Studio: V3051 Eine übermäßige Typprüfung. Das Objekt ist bereits vom Typ 'Ausnahme'. SmbFileInputStream.cs 107

 protected internal virtual IOException SeToIoe(SmbException se) { IOException ioe = se; Exception root = se.GetRootCause(); if (root is TransportException) { ioe = (TransportException)root; root = ((TransportException)ioe).GetRootCause(); } if (root is Exception) { ioe = new IOException(root.Message); ioe.InitCause(root); } return ioe; }

Und hier ist offen gesagt nicht ganz klar, was der Entwickler generell erreichen wollte. Der Analyzer warnt, dass die zweite if-Anweisung das Stammobjekt auf Kompatibilität mit seinem eigenen Typ überprüft. Höchstwahrscheinlich ist dies nur redundanter Code, aber er sieht verdächtig aus. Der Entwickler sollte ihn sich genauer ansehen.

Fazit


In jeder Hinsicht haben die Entwickler bei der Entwicklung von Emby großartige Arbeit geleistet (das Projekt enthält 215.539 Codezeilen, 4,6% - Kommentare). Sie sind wirklich großartig. Der Analysator hat es jedoch auch versucht, indem er Warnungen mit den Pegeln " Hoch - 113", "Mittel - 213" und "Niedrig - 112" ausgegeben hat.

Beispielsweise wurden allein bei den Warnungen V3022 (der Ausdruck ist immer falsch / wahr) 106 Punkte erzielt. Sie können natürlich überprüfen, welcher Teil von ihnen falsch positiv war, und den Rest dem Artikel hinzufügen, aber dies führt zu einem sehr langweiligen Text, den niemand lesen wird.

Ich hoffe, es ist mir gelungen, die Nützlichkeit des statischen Analysators in der Entwicklung zu demonstrieren. Einmalige Überprüfungen reichen natürlich nicht aus, und ein statischer Analysator muss ständig verwendet werden. Weitere Informationen hierzu finden Sie im Artikel Godot: Zur regelmäßigen Verwendung von Static Code Analyzers .



Wenn Sie diesen Artikel mit einem englischsprachigen Publikum teilen möchten, verwenden Sie bitte den Link zur Übersetzung: Ekaterina Nikiforova. Emby mit PVS-Studio überprüfen .

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

More articles:


All Articles