Geekly articles weekly

使用PVS-Studio分析器嵌入源代码分析

图片3

Emby与Plex和Kodi一起是相当知名的媒体服务器。 在本文中，我们将尝试使用PVS-Studio静态分析器验证其源代码。 在Emby官方网站上，有一个小标记“ Built with ReSharper”。 好吧，它甚至更有趣。

PVS-Studio代码分析器


PVS-Studio在Windows，Linux和macOS的64位系统上运行。 他知道如何在用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而不检查null ，这可能导致引发异常。

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行代码上发出了多达13个分析器警告（比第二行多一点）。 因此，我认为该代码应包含在本文中。 接下来，我将写下分析仪特别要注意的内容。

  1. 首先，考虑表达式frame.IsCompressed && _compression == CompressionMethod.None ，如果表达式为true，则将执行processUnsupportedFrame方法，该方法在任何情况下均返回false （因此，分析器将首先发出警告）。 如果为假，则继续进行下一点。
  2. 接下来， 检查frame.IsFragmented的值。 没问题，让我们继续。
  3. 这里检查frame.IsData的值。 如果为true，则processDataFrame方法将始终返回true 。 因此，第二个分析仪警告。
  4. 然后我们检查frame.IsPing 。 True- processPingFrame将返回true 。 这是分析仪的第三个警告。
  5. 我们看一下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，然后立即在if语句中检查此变量？

也许这段代码只是多余的。 无论如何，都值得仔细检查。

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

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

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

现在，我将解释为什么分析器认为代码中的RefreshStarted调用不安全。 假设在检查null和直接在if主体中调用事件处理程序之间的时间，从另一个线程中的事件取消订阅。 并且，如果没有更多订阅者，则RefreshStarted将为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语句 。 if语句的主体不同。 很难说这是一个错误还是多余的代码。 也许一切都很好，并且作者只是想在逻辑上分离两个不同的动作，其中一个与某个“徽标”相关联，而第二个与某个“艺术”相关联。

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行代码，占4.6％-注释）。 他们真的很棒。 但是，分析仪还尝试通过发出高-113，中-213和低-112 级别的警告来进行警告，其中一些是误报，但大多数错误并未出现在文章中，因为它们看起来很像。

例如， 仅警告V3022 （表达式始终为false / true）得分为106件。 当然，您可以检查其中的哪一部分是误报，然后将其余部分添加到文章中，但这将导致非常无聊的文本，没人会读。

我希望我能证明静态分析器在开发中的有用性。 当然，仅进行一次检查是不够的，必须持续使用静态分析仪。 您可以在Godot中阅读有关此内容的更多信息：关于静态代码分析器的常规使用 。



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

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

More articles:


All Articles