Geekly articles weekly

使用PVS-Studio检查Emby

图片3

Emby与Plex和Kodi一起是颇受欢迎的媒体服务器。 在本文中，我们将与静态分析器PVS-Studio讨论在其源代码中发现的错误。 该项目官方网站上的备注“与ReSharper一起构建”使分析更加有趣。

PVS工作室


PVS-Studio在64位Windows，Linux和macOS系统上运行。 它可以检测用C，C ++，C＃和Java编写的软件的源代码中的错误。

正在分析的项目


Emby是媒体服务器； 其源代码可在GitHub上获得 。 它允许用户在任何设备上流式传输和访问其媒体内容（视频，音频，照片）。 根据项目的官方网站，这是Emby的功能的简要摘要：

  • Emby即时自动转换和流式传输媒体，以在任何设备上播放；
  • 广泛的父母控制选项可简化内容访问控制，这对有小孩的家庭来说是一个重要功能；
  • Emby将您的内容整理成简单而优雅的演示文稿。 您的个人媒体永远不会一样。
  • 流媒体支持云同步；
  • 与您的朋友和家人轻松共享内容；
  • 还有更多。

分析器报告的最有趣的代码段


PVS-Studio诊断消息： V3001在'&&'运算符的左侧和右侧有相同的子表达式'c！='<''。 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; .... }

分析器检测到重复的子表达式c！='<' 。 一种解释是这是一个编程错误，开发人员打算用其他东西代替'<' 。 另一个更可能的解释是，第二个子表达式根本不存在，应将其删除。

PVS-Studio诊断消息： V3001在“ |”的左侧和右侧有相同的子表达式“ SmbConstants.AttrHidden” 操作员。 SmbComDelete.cs 29

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

与重复的子表达式有关的另一种错字。 顺便说一句，Emby的源代码中有太多这样的问题-由于注意力不集中而导致的错误。 我不是在责怪开发人员； 我们有时可能会心不在（（ 示例 ， 示例 ， 示例 ），这正是存在静态分析的原因-保护我们免受自己的错误的影响。

PVS-Studio诊断消息： V3004'then '语句等效于'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 + "))"; } .... }

这个看起来非常像一个复制粘贴错误，因为if和else块具有相同的主体。 如果不影响后续执行逻辑，则检查typesToCount数组的大小有什么好处 ？ 这是只有作者知道的东西。

PVS-Studio诊断消息： V3005已将 '_validProviderIds'变量分配给它自己。 BaseNfoParser.cs 77

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

另一个错字，导致为变量分配自己的值。 该代码需要修改。

PVS-Studio诊断消息： V3008 “章节”变量已连续两次分配值。 也许这是一个错误。 检查行：29，28。Title.cs 29

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

这又是关于注意力不集中和错别字的问题。...为Chapters变量分配了两次值。 当然，这种重复的分配不会造成任何伤害，但是您仍然不希望在代码中出现类似的事情。 没有意义在这上面徘徊，让我们继续前进。

PVS-Studio诊断消息： V3013奇怪的是，“读取”功能的主体与“写入”功能的主体完全相同 （407，第415行）。 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; } } }

读取和写入功能具有相同的主体，这就是分析器告诉我们的内容。 EnterWriteLock方法用于在写模式下输入锁。 如果要在读取模式下输入锁，请使用EnterReadLock方法，该方法允许一次由多个线程读取资源。

开发人员应检查此代码，因为它很可能包含错误-更重要的是，由于在代码中发现了一个未使用的类，因此：

 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诊断消息： V3021有两个带有相同条件表达式的'if'语句。 第一个'if'语句包含方法return。 这意味着第二个'if'语句是毫无意义的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"); } .... }

开发人员必须已经克隆了前四行，但是忘记将要检查的变量的名称从inputPath更改为outputPath 。 关于在没有事先进行空检查的情况下使用outputPath的地方，还有几行内容，这意味着可能会引发异常。

PVS-Studio诊断消息：

  • V3022表达式'processUnsupportedFrame（frame，CloseStatusCode.PolicyViolation，null）'始终为false。 WebSocket.cs 462
  • V3022表达式'processCloseFrame（frame）'始终为false。 WebSocket.cs 461
  • V3022表达式'frame.IsClose？ processCloseFrame（frame）：processUnsupportedFrame（frame，CloseStatusCode.PolicyViolation，null）'始终为false。 WebSocket.cs 460
  • V3022表达式'processPongFrame（frame）'始终为true。 WebSocket.cs 459
  • V3022表达式'processPingFrame（frame）'始终为true。 WebSocket.cs 457
  • V3022表达式'processDataFrame（frame）'始终为true。 WebSocket.cs 455
  • V3022表达式始终为假。 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; }

我到目前为止检查的项目少于我的PVS-Studio队友，这也许可以解释为什么我以前从未见过13行的代码片段会立即触发7条警告（即每两行略多于一条警告）。 这就是为什么我将这种情况纳入文章。 以下是问题片段的分步分析。

  1. 表达式frame.IsCompressed && _compression == CompressionMethod.None首先被求值。 如果为true，则无论如何都将执行processUnsupportedFrame方法并返回false （这是第一个警告）。 如果检查为false ，我们继续进行下一个检查。
  2. 值frame.IsFragmented被检查。 这里没有问题。
  3. 值frame.IsData已检查。 如果为true，则在任何情况下processDataFrame方法都将返回true 。 这是第二个警告。
  4. 将检查值frame.IsPing 。 如果为true，则processPingFrame方法将返回true 。 这是第三个警告。
  5. 将检查value frame.IsPong 。 与上一个相同。
  6. 最后一个： frame.IsClose 。 在任何情况下， processCloseFrame和processUnsupportedFrame均返回false 。

图片1

我希望不要太繁琐。 其余示例并不那么复杂。

PVS-Studio诊断消息： V3085嵌套类型中的“ RtpHeaderBytes”字段的名称不明确。 外部类型包含名称相同的静态字段。 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; .... } .... }

嵌套类UdpClientStream的字段名称与包含的类HdHomerunUdpStream的名称相同。 这不是错误，但这是再次检查此代码以确保正确的充分理由。 使用具有相同名称的变量可以很容易地意外地使用其中一个而不是另一个变量，从而导致程序出现意外行为，而编译器不会说一个字。

PVS-Studio诊断消息：

  • V3090对类型的不安全锁定。 一个类型的所有实例将具有相同的“类型”对象。 Lmhosts.cs 49
  • V3090对类型的不安全锁定。 一个类型的所有实例将具有相同的“类型”对象。 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)) { .... } } }

分析仪在此警告不安全的锁。 不建议以这种方式使用锁 ，因为锁对象是可以公开访问的，并且可以在其他地方锁定，并且最初使用此对象的开发人员可能永远都不知道。 这可能导致死锁。

理想情况下，您应该使用私有字段进行锁定，例如：

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

PVS-Studio诊断消息： V3142检测到无法识别的代码。 可能存在错误。 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(....); }

分析器说，此代码段中的最后一行将永远不会执行。 声明变量enableHttpStream，为其分配true并随后进行检查的目的是什么？

也许这段代码只是多余的，但无论如何都需要修改。

PVS-Studio诊断消息： V3083对事件'RefreshStarted'的不安全调用，可能为NullReferenceException。 请考虑在调用事件之前将事件分配给局部变量。 ProviderManager.cs 943

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

分析器会警告我们有关RefreshStarted事件处理程序的潜在不安全调用。

让我们弄清楚为什么此调用不安全。 假设在检查事件为null以及在if语句的主体中调用事件处理程序之间的时刻，该事件不再在另一个线程中订阅。 如果没有订阅者，则RefreshStarted事件将变为null ，但是在已经通过了null检查的线程中，无论如何将执行调用：

 RefreshStarted(this, new GenericEventArgs<BaseItem>(item));

这将导致抛出NullReferenceException 。

PVS-Studio诊断消息： V3029彼此并排的'if'语句的条件表达式相同。 检查行：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(....); } .... }

这两个if语句具有相同的条件，但是它们的主体不同。 我不确定这是错误还是只是冗余代码。 也许还可以，并且开发人员只想明确区分两个动作，其中一个动作与“ Logo”有关，另一个与“ Art”有关，无论它们是什么。

PVS-Studio诊断消息： V3041该表达式从'int'类型隐式转换为'double'类型。 考虑使用显式类型转换以避免丢失小数部分。 例如：double A =（double）（X）/ Y;。 LiveTvManager.cs 1085

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

这段代码包含一个整数除法，其结果值被强制转换为浮点类型，这似乎不是正确的选择。

实际上，仅当_services.Length = 1时 ， progressPerService变量的值才为1.0。 使用_services.Length的任何其他值，结果将为0.0 。

我认为应该写的是以下内容：

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

PVS-Studio诊断消息： V3050可能是错误的HTML。 遇到</ u>结束标记，而期望</ i>标记。 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" }, .... } }; }

注意这一行：

 <u>underline</u>

它已经有一个结束标记</ u> 。
然后我们看到以下文本：
 </s></u></i></b> HTML tags"

分析器指出，这里还有一个额外的结束标记</ u> 。

PVS-Studio诊断消息： V3051类型检查过多。 该对象已经是“异常”类型。 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; }

坦白说，我不太了解开发人员使用此代码的含义。 分析器说第二个if语句的条件检查根对象是否与其自己的类型兼容。 这可能只是冗余代码，但看起来确实很奇怪，我建议对其进行修改。

结论


Emby的开发人员在各个方面都做得很好（该项目的时长为215539 ​​LOC，其中4.6％是评论）。 他们做得很好，我是说真的。 但是PVS-Studio也值得赞扬：它产生了113个高级别 ，213个中级别和112个低级别警告。 其中一些是误报，但此处未提及大多数错误，因为它们几乎相同。 例如， 仅V3022诊断（始终为假/真状态）被触发了106次。 当然，我本可以过滤掉误报，然后将其余内容纳入文章，但阅读起来太无聊了。

我希望我能证明静态分析如何帮助软件开发。 显然，仅进行一次检查是不够的。 您应该定期使用静态分析。 在“ Godot：静态分析仪的常规使用 ”一文中将更详细地讨论该主题。

Source: https://habr.com/ru/post/zh-CN484046/

More articles:


All Articles