Geekly articles weekly

Análise de código fonte Emby com o analisador PVS-Studio

Quadro 3

Emby é um servidor de mídia bastante conhecido, juntamente com Plex e Kodi. Neste artigo, tentaremos verificar seu código-fonte usando o analisador estático PVS-Studio. No site oficial da Emby, há uma pequena marca "Built with ReSharper". Bem, é ainda mais interessante.

Analisador de código PVS-Studio


O PVS-Studio é executado em sistemas de 64 bits no Windows, Linux e macOS. Ele sabe como procurar erros no código fonte dos programas escritos em C, C ++, C # e Java.

Projeto Auditado


Emby é um servidor de mídia cujo código fonte pode ser encontrado no GitHub . Permite organizar transmissões e possibilita o acesso a conteúdo de mídia (vídeo, áudio, fotos) de qualquer dispositivo. Aperte a partir do site oficial da Emby :

  • O Emby converte automaticamente sua mídia rapidamente para reprodução em qualquer dispositivo;
  • O amplo controle dos pais facilita o controle do acesso ao conteúdo, o que será relevante para famílias com filhos pequenos;
  • Emby organiza sua mídia em apresentações simples e elegantes. Eles nunca terão a mesma aparência;
  • Streaming de dados com sincronização na nuvem;
  • Compartilhe facilmente seus arquivos de mídia com amigos e familiares;
  • E muito mais

Partes do código que chamaram a atenção ao examinar um relatório do analisador


PVS-Studio Warning: V3001 Existem sub-expressões idênticas 'c! =' <'' À esquerda e à direita do operador '&&'. 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; .... }

O analisador detectou uma subexpressão duplicada c! = '<' . Talvez tenha ocorrido um erro nesse código, e algo mais deveria estar no lugar de '<' . Ou, mais provavelmente, a segunda subexpressão é supérflua e você só precisa excluí-la.

PVS-Studio Warning: V3001 Existem sub-expressões idênticas 'SmbConstants.AttrHidden' à esquerda e à direita da '|' operador. SmbComDelete.cs 29

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

O analisador encontrou novamente um erro de digitação associado a subexpressões repetidas. Olhando para o futuro, direi que neste projeto há muitos erros semelhantes cometidos por descuido. Eu não censuro os criadores de Emby, às vezes somos dispersos dessa maneira ( exemplos , exemplos , exemplos ) e, para estar seguro de tais erros, existe apenas uma análise estática.

PVS-Studio Warning: V3004 A instrução 'then' é equivalente à instrução '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 + "))"; } .... }

E isso é muito parecido com o problema que apareceu devido a copiar e colar, porque if e else os corpos dos blocos são completamente idênticos. Por que você precisou verificar os tamanhos da matriz typesToCount se, independentemente do número de elementos escritos nela, o mesmo código será executado - é conhecido apenas pelos desenvolvedores.

PVS-Studio Warning: V3005 A variável '_validProviderIds' é atribuída a si mesma. BaseNfoParser.cs 77

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

Novamente, um erro de digitação, que implicava a atribuição de uma variável a si mesma. Vale a pena conferir esta seção de código.

PVS-Studio Warning: V3008 A variável 'Chapters' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 29, 28. Title.cs 29

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

Desatenção e erros de digitação, novamente ... A variável Capítulos recebe um valor duas vezes. É claro que esse código extra não ameaça nada de ruim, mas mesmo assim. Não faz sentido continuar nesse exemplo, vamos seguir em frente.

PVS-Studio Warning: V3013 É estranho que o corpo da função 'Read' seja totalmente equivalente ao corpo da função 'Write' (407, linha 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; } } }

Os corpos das funções de leitura e gravação são absolutamente idênticos, conforme evidenciado pelo texto de aviso do analisador. O método EnterWriteLock é necessário para inserir o bloqueio no modo de gravação. Para o modo de leitura, vale a pena usar o método EnterReadLock , que permite a capacidade de ler um recurso simultaneamente em vários threads.

O desenvolvedor deve dar uma olhada mais de perto nesta seção do código, talvez haja um erro. Além disso, a seguinte classe foi encontrada no código, que não é usado:

 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 Warning: V3021 Existem duas instruções 'if' com expressões condicionais idênticas. A primeira instrução 'if' contém retorno de método. Isso significa que a segunda declaração 'if' não faz sentido. 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"); } .... }

Aqui, aparentemente, o desenvolvedor copiou as quatro primeiras linhas, mas esqueceu de alterar a variável que está sendo testada de inputPath para outputPath . Existem várias linhas no código que usam outputPath sem verificar nulo , o que pode levar a uma exceção.

Avisos do PVS-Studio:

  • A expressão V3022 'processUnsupportedFrame (quadro, CloseStatusCode.PolicyViolation, null)' sempre é falsa. WebSocket.cs 462
  • A expressão V3022 'processCloseFrame (frame)' é sempre falsa. WebSocket.cs 461
  • V3022 Expression 'frame.IsClose? processCloseFrame (frame): processUnsupportedFrame (frame, CloseStatusCode.PolicyViolation, null) 'é sempre falso. WebSocket.cs 460
  • A expressão V3022 'processPongFrame (quadro)' é sempre verdadeira. WebSocket.cs 459
  • A expressão V3022 'processPingFrame (frame)' é sempre verdadeira. WebSocket.cs 457
  • A expressão V3022 'processDataFrame (frame)' é sempre verdadeira. WebSocket.cs 455
  • A expressão V3022 é sempre falsa. 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; }

Na minha conta, não existem muitos, em comparação com o restante da equipe do PVS-Studio, verificados pelo analisador de projetos, e talvez isso esteja relacionado ao fato de eu ver pela primeira vez que até 13 avisos do analisador são emitidos em 13 linhas de código (um pouco mais do que cada segunda linha). Portanto, acredito que esse código merece ser incluído no artigo. A seguir, anotarei o que o analisador jura especificamente.

  1. Primeiro, a expressão frame.IsCompressed && _compression == CompressionMethod.None é considerada e, se a expressão for verdadeira, o método processUnsupportedFrame será executado, o que, em qualquer caso, retorna falso (daí o primeiro aviso do analisador). Se fosse falso, passe para o próximo ponto.
  2. Em seguida, o valor de frame.IsFragmented é verificado . Não há problemas, então vamos seguir em frente.
  3. Aqui, o valor de frame.IsData é verificado . Se verdadeiro, o método processDataFrame retornará verdadeiro de qualquer maneira. Daí o segundo aviso do analisador.
  4. Então, verificamos o frame.IsPing . True - processPingFrame retornará true . Este é o terceiro aviso do analisador.
  5. Olhamos para o frame.IsPong . Tudo é semelhante ao anterior.
  6. E o último: frame.IsClose . processCloseFrame e processUnsupportedFrame, em qualquer caso, retornam false .

Quadro 1

Espero não ter aborrecido você. Será mais simples ainda.

PVS-Studio Warning: V3085 O nome do campo 'RtpHeaderBytes' em um tipo aninhado é ambíguo. O tipo externo contém campo estático com nome idêntico. 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; .... } .... }

A classe aninhada UdpClientStream usa um campo com um nome semelhante ao usado na classe externa HdHomerunUdpStream . Isso não é um erro, mas é um bom motivo para verificar se está tudo bem com este código. Seu problema é que há uma chance de confundir essas duas variáveis, e o código não funcionará como pretendido pelo desenvolvedor, embora não cause reclamações do compilador.

Avisos do PVS-Studio:

  • V3090 Bloqueio inseguro em um tipo. Todas as instâncias de um tipo terão o mesmo objeto 'Tipo'. Lmhosts.cs 49
  • V3090 Bloqueio inseguro em um tipo. Todas as instâncias de um tipo terão o mesmo objeto 'Tipo'. 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)) { .... } } }

O analisador avisa que um bloqueio não seguro é usado neste fragmento de código. O uso de bloqueio é indesejável porque o objeto usado para o bloqueio é compartilhado e pode ser usado para bloquear em outro local sem o conhecimento do desenvolvedor que usou esse objeto pela primeira vez. O resultado será um impasse no mesmo objeto.

Idealmente, você deve usar um campo privado para bloquear, por exemplo, assim:

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

Aviso do PVS-Studio: V3142 Código inacessível detectado. É possível que haja um erro. 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(....); }

O analisador indica que a última linha deste fragmento de código nunca será executada. E por que o desenvolvedor declara e define a variável enableHttpStream como true e verifica imediatamente essa variável na instrução if?

Talvez esse código seja simplesmente redundante. De qualquer forma, vale a pena checar novamente.

PVS-Studio Warning: V3083 Chamada não segura do evento 'RefreshStarted', NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. ProviderManager.cs 943

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

Esse método usa uma chamada potencialmente insegura para o manipulador de eventos RefreshStarted , como indica o analisador.

Agora vou explicar por que o analisador considerou esta chamada RefreshStarted no código inseguro. Suponha que, no momento entre a verificação de nulo e a chamada direta do manipulador de eventos no corpo if , seja realizada uma desinscrição do evento em outro encadeamento. E, se não houver mais assinantes, o RefreshStarted será nulo . Mas em outro encadeamento, onde uma verificação nula já ocorreu, a seguinte linha será executada:
 RefreshStarted(this, new GenericEventArgs<BaseItem>(item));

Isso lançará uma NullReferenceException .

PVS-Studio Warning: V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 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(....); } .... }

Nesse trecho de código, um ao lado do outro, existem duas instruções if com as mesmas condições. Os corpos das instruções if são diferentes. Difícil dizer se isso é um erro ou um código redundante. Talvez esteja tudo bem e o autor só queira separar logicamente duas ações diferentes, uma das quais está associada a um certo "logotipo" e a segunda a uma certa "arte".

PVS-Studio Warning: V3041 A expressão foi convertida implicitamente do tipo 'int' para o tipo 'double'. Considere utilizar uma conversão de tipo explícita para evitar a perda de uma parte fracionária. Um exemplo: double A = (double) (X) / Y; LiveTvManager.cs 1085

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

Este código contém a operação de divisão de tipos de dados inteiros. O valor resultante é implicitamente convertido em um tipo de ponto flutuante, o que é extremamente suspeito.

Na verdade, a variável progressPerService será igual a 1.0 somente se _services.Length = 1 . Para outros valores de _services.Length , o resultado será 0,0 .

Aparentemente, deve ser escrito aqui:

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


PVS-Studio Warning: V3050 Possivelmente um HTML incorreto. A tag de fechamento </u> foi encontrada, enquanto a tag </i> era esperada. 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" }, .... } }; }

Preste atenção na linha

 <u>underline</u>

Ele já tem uma tag de fechamento </u> . Em seguida, vemos este texto:
 </s></u></i></b> HTML tags"

Aqui a tag de fechamento </u> é supérflua, como indica o analisador.
Aviso PVS-Studio: V3051 Uma verificação de tipo excessiva. O objeto já é do tipo 'Exceção'. 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; }

E aqui, francamente, não está totalmente claro o que o desenvolvedor geralmente queria alcançar. O analisador avisa que a segunda instrução if verifica a compatibilidade do objeto raiz com seu próprio tipo. Provavelmente, este é apenas um código redundante, mas parece suspeito, o desenvolvedor deve dar uma olhada mais de perto.

Conclusão


Em todos os sentidos, os desenvolvedores fizeram um ótimo trabalho ao criar o Emby (o projeto possui 215 539 linhas de código, 4,6% - comentários). Eles são ótimos, realmente. No entanto, o analisador também tentou emitir avisos dos níveis Alto - 113, Médio - 213 e Baixo - 112. Alguns deles eram falsos positivos, mas a maioria dos erros não apareceu no artigo porque eram parecidos.

Por exemplo, os avisos V3022 sozinhos (a expressão é sempre falsa / verdadeira) pontuaram 106 peças. Obviamente, você pode verificar qual parte deles era falsa-positiva e adicionar o restante ao artigo, mas isso resultará em um texto muito chato que ninguém lerá.

Espero ter conseguido demonstrar a utilidade do analisador estático no desenvolvimento. Obviamente, as verificações únicas não são suficientes e um analisador estático deve ser usado continuamente. Você pode ler mais sobre isso no artigo Godot: Sobre o uso regular de analisadores de código estático .



Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Ekaterina Nikiforova. Verificando o Emby com o PVS-Studio .

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

More articles:


All Articles