WinForms: erros, Holmes

Quadro 5

Adoramos procurar bugs em projetos da Microsoft. Porque É simples: seus projetos geralmente são fáceis de verificar (o trabalho pode ser feito imediatamente no ambiente do Visual Studio, para o qual o PVS-Studio possui um plug-in conveniente) e eles contêm poucos erros. Portanto, o algoritmo usual de trabalho é o seguinte: encontre e baixe um projeto aberto do MS; confira; escolha erros interessantes; certifique-se de que são poucos; escreva um artigo sem esquecer de elogiar os desenvolvedores. Ótimo! Ganha-ganha-ganha: demorou um pouco, o manual tem prazer em ver novos materiais no blog e o karma está em perfeita ordem. Mas desta vez algo deu errado. Vamos ver o que foi encontrado interessante no código fonte do Windows Forms e se a Microsoft deve ser elogiada desta vez.

1. Introdução

No início de dezembro de 2018, a Microsoft anunciou o lançamento do .NET Core 3 Preview 1. Um pouco antes (aproximadamente de meados de outubro), o GitHub começou a trabalhar ativamente na publicação do código-fonte do Windows Forms , a plataforma de interface do usuário do .NET Core para criar aplicativos de desktop do Windows. Você pode ver as estatísticas de confirmação aqui . Agora qualquer pessoa pode baixar o código fonte do WinForms para revisão.

Também baixei as fontes para procurar erros usando o PVS-Studio. O cheque não causou dificuldades. Necessário: Visual Studio 2019, Visualização do SDK do .NET Core 3.0, PVS-Studio. E agora o log de aviso do analisador é recebido.

Depois de receber o log do PVS-Studio, costumo classificá-lo em ordem crescente de números de diagnóstico (a janela com o log de mensagens do PVS-Studio no Visual Studio possui várias opções para classificar e filtrar a lista). Isso permite que você trabalhe com grupos de erros do mesmo tipo, o que simplifica bastante a análise do código fonte. Eu marquei erros interessantes na lista com um asterisco e só então, depois de analisar o log inteiro, escrevo fragmentos de código e faço uma descrição. E, como geralmente há poucos erros, eu os misturo, tentando colocar o mais interessante no início e no final do artigo. Mas desta vez, os erros acabaram sendo um pouco demais (eh, não foi possível salvar a intriga por muito tempo), e eu os trarei na ordem dos números de diagnóstico.

O que foi encontrado? 833 avisos do nível de confiança Alto e Médio (249 e 584, respectivamente) foram emitidos para 540.000 linhas de código (excluindo as vazias) em arquivos de 1670 cs. E sim, de acordo com a tradição, não verifiquei os testes e não considerei os avisos do baixo nível de confiança (foram emitidos 215). De acordo com minhas observações anteriores, existem muitos avisos para o projeto do MS. Mas nem todos os avisos são erros.

Para este projeto, o número de falsos positivos foi de cerca de 30%. Em cerca de 20% dos casos, eu simplesmente não consegui concluir exatamente se isso é um erro ou não, pois não conheço bem o código. Bem, pelo menos 20% dos erros que eu perdi podem ser atribuídos ao fator humano: pressa, fadiga, etc. O efeito oposto também é possível: a propósito, observei alguns dos mesmos tipos de disparos, cujo número poderia chegar a 70-80, ou outro que ocasionalmente poderia aumentar o número de erros que eu considerava reais.

De qualquer forma, 30% dos avisos indicam erros reais, uma porcentagem muito grande, pois o analisador não foi pré-configurado.

Portanto, o número de erros que eu consegui detectar era de cerca de 240, o que está dentro dos limites das estatísticas. Repito, na minha opinião, para um projeto da MS, esse não é o resultado mais destacado (embora sejam apenas 0,44 erros por 1.000 linhas de código) e provavelmente haja mais erros reais no código WinForms. Proponho falar sobre os motivos no final do artigo, mas agora vamos ver os erros mais interessantes.

Erros

PVS-Studio: V3003 O uso do padrão 'if (A) {...} else if (A) {...}' foi detectado. Há uma probabilidade de presença de erro lógico. Verifique as linhas: 213, 224. ButtonStandardAdapter.cs 213

void PaintWorker(PaintEventArgs e, bool up, CheckState state) { up = up && state == CheckState.Unchecked; .... if (up & IsHighContrastHighlighted()) { .... } else if (up & IsHighContrastHighlighted()) { .... } else { .... } .... } 

Nos blocos if e else if , a mesma condição é verificada. Parece copiar e colar. Isso é um erro? Se você observar a declaração do método IsHighContrastHighlighted , há dúvidas:

 protected bool IsHighContrastHighlighted() { return SystemInformation.HighContrast && Application.RenderWithVisualStyles && (Control.Focused || Control.MouseIsOver || (Control.IsDefault && Control.Enabled)); } 

O método provavelmente pode retornar valores diferentes em chamadas sucessivas. E o que é feito no método de chamada parece estranho, é claro, mas tem direito à vida. No entanto, eu aconselho os autores a dar uma olhada neste pedaço de código. Apenas no caso. E, no entanto, este é um bom exemplo de quão difícil é tirar conclusões ao analisar código desconhecido.

PVS-Studio: V3004 A instrução 'then' é equivalente à instrução 'else'. RichTextBox.cs 1018

 public int SelectionCharOffset { get { int selCharOffset = 0; .... NativeMethods.CHARFORMATA cf = GetCharFormat(true); // if the effects member contains valid info if ((cf.dwMask & RichTextBoxConstants.CFM_OFFSET) != 0) { selCharOffset = cf.yOffset; // <= } else { // The selection contains characters of different offsets, // so we just return the offset of the first character. selCharOffset = cf.yOffset; // <= } .... } .... } 

E aqui o erro de copiar e colar é definitivamente feito. Independentemente da condição, a variável selCharOffset sempre obterá o mesmo valor.

Havia mais dois erros semelhantes no código WinForms:
  • V3004 A instrução 'then' é equivalente à instrução 'else'. SplitContainer.cs 1700
  • V3004 A instrução 'then' é equivalente à instrução 'else'. ToolstripProfessionalRenderer.cs 371

PVS-Studio: V3008 A variável recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 681, 680. ProfessionalColorTable.cs 681

 internal void InitSystemColors(ref Dictionary<KnownColors, Color> rgbTable) { .... rgbTable[ProfessionalColorTable.KnownColors.msocbvcrCBBdrOuterDocked] = buttonFace; rgbTable[ProfessionalColorTable.KnownColors.msocbvcrCBBdrOuterDocked] = buttonShadow; .... } 

O método preenche o dicionário rgbTable . O analisador apontou para um pedaço de código no qual dois valores diferentes são gravados seqüencialmente na mesma chave. E tudo ficaria bem, mas havia mais 16 lugares nesse método, o que não parece mais um único erro. Mas por que fazer isso, para mim, continua sendo um mistério. Não encontrei sinais de código gerado automaticamente. No editor, fica assim:

Quadro 3

Vou dar as dez primeiras operações com uma lista:

  1. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 785, 784. ProfessionalColorTable.cs 785
  2. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 787, 786. ProfessionalColorTable.cs 787
  3. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 789, 788. ProfessionalColorTable.cs 789
  4. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 791, 790. ProfessionalColorTable.cs 791
  5. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 797, 796. ProfessionalColorTable.cs 797
  6. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 799, 798. ProfessionalColorTable.cs 799
  7. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 807, 806. ProfessionalColorTable.cs 807
  8. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 815, 814. ProfessionalColorTable.cs 815
  9. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 817, 816. ProfessionalColorTable.cs 817
  10. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 823, 822. ProfessionalColorTable.cs 823

PVS-Studio: V3011 Foram encontradas duas condições opostas. A segunda condição é sempre falsa. Verifique as linhas: 5242, 5240. DataGrid.cs 5242

 private void CheckHierarchyState() { if (checkHierarchy && listManager != null && myGridTable != null) { if (myGridTable == null) // <= { // there was nothing to check return; } for (int j = 0; j < myGridTable.GridColumnStyles.Count; j++) { DataGridColumnStyle gridColumn = myGridTable.GridColumnStyles[j]; } checkHierarchy = false; } } 

A declaração de retorno nunca será executada. Provavelmente, a condição myGridTable! = Nulo no externo se o bloco foi adicionado posteriormente durante a refatoração. E agora verificar myGridTable == null não faz sentido. Para melhorar a qualidade do código, remova esta verificação.

PVS-Studio: V3019 Possivelmente uma variável incorreta é comparada a nula após a conversão do tipo usando a palavra-chave 'as'. Verifique as variáveis ​​'left', 'cscLeft'. TypeCodeDomSerializer.cs 611

PVS-Studio: V3019 Possivelmente uma variável incorreta é comparada a nula após a conversão do tipo usando a palavra-chave 'as'. Verifique as variáveis ​​'right', 'cscRight'. TypeCodeDomSerializer.cs 615

 public int Compare(object left, object right) { OrderedCodeStatementCollection cscLeft = left as OrderedCodeStatementCollection; OrderedCodeStatementCollection cscRight = right as OrderedCodeStatementCollection; if (left == null) { return 1; } else if (right == null) { return -1; } else if (right == left) { return 0; } return cscLeft.Order - cscRight.Order; // <= } 

O analisador emitiu imediatamente dois avisos para o método Compare . Qual é o problema? É que os valores de cscLeft e cscRight não são verificados quanto à igualdade nula . Eles podem obter esse valor após a conversão sem êxito no tipo OrderedCodeStatementCollection . Em seguida, uma exceção será lançada na última declaração de retorno . Essa situação é possível quando todas as verificações para aprovação da esquerda e da direita e não levam a uma saída preliminar do método.

Para corrigir o código, cscLeft / cscRight deve ser usado em qualquer lugar, em vez de esquerdo / direito .

PVS-Studio: V3020 Uma 'quebra' incondicional dentro de um loop. SelectionService.cs 421

 void ISelectionService.SetSelectedComponents( ICollection components, SelectionTypes selectionType) { .... // Handle the click case object requestedPrimary = null; int primaryIndex; if (fPrimary && 1 == components.Count) { foreach (object o in components) { requestedPrimary = o; if (o == null) { throw new ArgumentNullException(nameof(components)); } break; } } .... } 

Este fragmento refere-se, antes, ao código "com cheiro". Não há erro aqui. Mas surgem dúvidas sobre a organização do ciclo foreach . Por que é necessário aqui - é claro: devido à necessidade de extrair os elementos da coleção, passados ​​como ICollection . Mas por que, no loop, originalmente projetado para uma única iteração (uma condição prévia é a presença de um único elemento na coleção de componentes ), era necessária uma rede de segurança adicional na forma de quebra ? Provavelmente, a resposta pode ser considerada: "Ele se desenvolveu historicamente". O código parece feio.

PVS-Studio: V3022 A expressão 'ocxState! = Null' sempre é verdadeira. AxHost.cs 2186

 public State OcxState { .... set { .... if (value == null) { return; } .... ocxState = value; if (ocxState != null) // <= { axState[manualUpdate] = ocxState._GetManualUpdate(); licenseKey = ocxState._GetLicenseKey(); } else { axState[manualUpdate] = false; licenseKey = null; } .... } } 

Devido a um erro lógico, um "código morto" foi formado nesse fragmento. Expressões no bloco else nunca serão executadas.

PVS-Studio: V3027 A variável 'e' foi utilizada na expressão lógica antes de ser verificada contra nula na mesma expressão lógica. ImageEditor.cs 99

 public override object EditValue(....) { .... ImageEditor e = ....; Type myClass = GetType(); if (!myClass.Equals(e.GetType()) && e != null && myClass.IsInstanceOfType(e)) { .... } .... } 

A variável e na condição é usada primeiro e depois verificada quanto à desigualdade nula . Olá, NullReferenceException .

Outro erro semelhante:

PVS-Studio: V3027 A variável 'dropDownItem' foi utilizada na expressão lógica antes de ser verificada contra nula na mesma expressão lógica. ToolStripMenuItemDesigner.cs 1351

 internal void EnterInSituEdit(ToolStripItem toolItem) { .... ToolStripDropDownItem dropDownItem = toolItem as ToolStripDropDownItem; if (!(dropDownItem.Owner is ToolStripDropDownMenu) && dropDownItem != null && dropDownItem.Bounds.Width < commitedEditorNode.Bounds.Width) { .... } .... } 

Uma situação semelhante à anterior, apenas com a variável dropDownItem . Eu acho que esses erros aparecem como resultado de desatenção durante a refatoração. Provavelmente parte da condição ! (DropDownItem.Owner é ToolStripDropDownMenu) foi adicionado ao código posteriormente.

PVS-Studio: V3030 Verificação recorrente. A condição 'columnCount> 0' já foi verificada na linha 3900. ListView.cs 3903

 internal ColumnHeader InsertColumn( int index, ColumnHeader ch, bool refreshSubItems) { .... // Add the column to our internal array int columnCount = (columnHeaders == null ? 0 : columnHeaders.Length); if (columnCount > 0) { ColumnHeader[] newHeaders = new ColumnHeader[columnCount + 1]; if (columnCount > 0) { System.Array.Copy(columnHeaders, 0, newHeaders, 0, columnCount); } .... } .... } 

Um erro que pode parecer inofensivo. De fato, bem, uma verificação extra é realizada, o que não afeta a lógica do trabalho. E às vezes eles fazem isso quando você precisa verificar novamente o status de um componente visual, por exemplo, obtendo o número de entradas na lista. Somente nesse caso, eles verificam a variável local columnCount . Isso é muito suspeito. Eles queriam verificar outra variável ou em uma das verificações eles usam a condição errada.

PVS-Studio: V3061 O parâmetro 'lprcClipRect' sempre é reescrito no corpo do método antes de ser usado. WebBrowserSiteBase.cs 281

 int UnsafeNativeMethods.IOleInPlaceSite.GetWindowContext( out UnsafeNativeMethods.IOleInPlaceFrame ppFrame, out UnsafeNativeMethods.IOleInPlaceUIWindow ppDoc, NativeMethods.COMRECT lprcPosRect, NativeMethods.COMRECT lprcClipRect, NativeMethods.tagOIFI lpFrameInfo) { ppDoc = null; ppFrame = Host.GetParentContainer(); lprcPosRect.left = Host.Bounds.X; lprcPosRect.top = Host.Bounds.Y; .... lprcClipRect = WebBrowserHelper.GetClipRect(); // <= if (lpFrameInfo != null) { lpFrameInfo.cb = Marshal.SizeOf<NativeMethods.tagOIFI>(); lpFrameInfo.fMDIApp = false; .... } return NativeMethods.S_OK; } 

Erro não óbvio. Sim, o parâmetro lprcClipRect é realmente inicializado com um novo valor, sem usá-lo de forma alguma. Mas no que isso resulta? Eu acho que em algum lugar no código de chamada, o link passado por esse parâmetro permanecerá inalterado, embora tenha sido planejado de maneira diferente. De fato, veja como trabalhar com outras variáveis ​​nesse método. Até o nome (o prefixo "Get") sugere que alguma inicialização será feita dentro do método através dos parâmetros passados. E assim é. Os dois primeiros parâmetros ( ppFrame e ppDoc ) são passados ​​com o modificador out e obtêm novos valores. Os links lprcPosRect e lpFrameInfo são usados ​​para acessar os campos da classe e inicializá-los. E apenas lprcClipRect sai da lista geral. Muito provavelmente, o modificador out ou ref é necessário para este parâmetro.

PVS-Studio: V3066 Possível ordem incorreta de argumentos passada para o método 'AdjustCellBorderStyle': 'isFirstDisplayedRow' e 'isFirstDisplayedColumn'. DataGridViewComboBoxCell.cs 1934

 protected override void OnMouseMove(DataGridViewCellMouseEventArgs e) { .... dgvabsEffective = AdjustCellBorderStyle( DataGridView.AdvancedCellBorderStyle, dgvabsPlaceholder, singleVerticalBorderAdded, singleHorizontalBorderAdded, isFirstDisplayedRow, // <= isFirstDisplayedColumn); // <= .... } 

O analisador suspeitou que os dois últimos argumentos foram confusos. Vamos dar uma olhada na declaração do método AdjustCellBorderStyle :

 public virtual DataGridViewAdvancedBorderStyle AdjustCellBorderStyle( DataGridViewAdvancedBorderStyledataGridViewAdvancedBorderStyleInput, DataGridViewAdvancedBorderStyle dataGridViewAdvancedBorderStylePlaceholder, bool singleVerticalBorderAdded, bool singleHorizontalBorderAdded, bool isFirstDisplayedColumn, bool isFirstDisplayedRow) { .... } 

Parece um erro. Sim, frequentemente alguns argumentos são passados ​​intencionalmente na ordem inversa, por exemplo, para trocar algumas variáveis. Mas não acho que esse seja exatamente o caso. Nada nos métodos de chamada ou chamado diz esse padrão de uso. Primeiro, as variáveis ​​do tipo bool são confusas. Em segundo lugar, os nomes dos métodos também são comuns: sem “Swap” ou “Reverse”. Além disso, não é tão difícil cometer um erro como esse. As pessoas geralmente percebem a ordem do par de linhas / colunas de maneira diferente. Para mim, por exemplo, apenas "linha / coluna" é familiar. Mas para o autor do método chamado AdjustCellBorderStyle , obviamente, a ordem mais familiar é "coluna / linha".

PVS-Studio: V3070 A variável não inicializada 'LANG_USER_DEFAULT' é usada ao inicializar a variável 'LOCALE_USER_DEFAULT'. NativeMethods.cs 890

 internal static class NativeMethods { .... public static readonly int LOCALE_USER_DEFAULT = MAKELCID(LANG_USER_DEFAULT); public static readonly int LANG_USER_DEFAULT = MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT); .... } 

Um erro raro. A ordem de inicialização dos campos da classe é misturada. Para calcular o valor do campo LOCALE_USER_DEFAULT , use o campo LANG_USER_DEFAULT , que ainda não foi inicializado e tem um valor 0. A propósito, a variável LANG_USER_DEFAULT não é usada em nenhum outro lugar do código. Eu não estava com preguiça e escrevi um pequeno programa de console que simula a situação. Em vez dos valores de algumas constantes usadas no código WinForms, substituí seus valores reais:

 internal static class NativeMethods { public static readonly int LOCALE_USER_DEFAULT = MAKELCID(LANG_USER_DEFAULT); public static readonly int LANG_USER_DEFAULT = MAKELANGID(0x00, 0x01); public static int MAKELANGID(int primary, int sub) { return ((((ushort)(sub)) << 10) | (ushort)(primary)); } public static int MAKELCID(int lgid) { return MAKELCID(lgid, 0x0); } public static int MAKELCID(int lgid, int sort) { return ((0xFFFF & lgid) | (((0x000f) & sort) << 16)); } } class Program { static void Main() { System.Console.WriteLine(NativeMethods.LOCALE_USER_DEFAULT); } } 

Como resultado do lançamento, o seguinte será exibido no console: 0. Agora, trocamos a declaração dos campos LOCALE_USER_DEFAULT e LANG_USER_DEFAULT . O resultado do programa desta forma: 1024. Acho que não há mais nada a comentar aqui.

PVS-Studio: V3080 Possível desreferência nula. Considere inspecionar 'ces'. CodeDomSerializerBase.cs 562

 protected void DeserializeStatement( IDesignerSerializationManager manager, CodeStatement statement) { .... CodeExpressionStatement ces = statement as CodeExpressionStatement; if (ces != null) { .... } else { .... DeserializeExpression(manager, null, ces.Expression); // <= .... } .... } 

O código que deve "cair" é estável o suficiente, porque você pode entrar no ramo else apenas quando a referência ces é nula .

Outro exemplo semelhante:

PVS-Studio: V3080 Possível desreferência nula. Considere inspecionar 'comboBox'. ComboBox.cs 6610

 public void ValidateOwnerDrawRegions(ComboBox comboBox, ....) { .... if (comboBox != null) { return; } Rectangle topOwnerDrawArea = new Rectangle(0, 0, comboBox.Width, innerBorder.Top); .... } 

O código paradoxal. Aparentemente, eles confundiram o teste escrevendo if (comboBox! = Null) em vez de if (comboBox == null) . E assim - receberemos a próxima NullReferenceException .

Examinamos dois erros bastante óbvios da V3080 , nos quais você pode acompanhar visualmente a situação do possível uso de referências nulas no método. Mas o diagnóstico do V3080 é muito mais inteligente e pode procurar erros semelhantes para cadeias de chamadas de método. Não faz muito tempo, fortalecemos significativamente os mecanismos de fluxo de dados e análise interprocedural. Você pode ler sobre isso no artigo " Tipos de referência anuláveis ​​no C # 8.0 e análise estática ". E aqui está um erro semelhante encontrado no WinForms:

PVS-Studio: V3080 Possível desreferência nula dentro do método em 'reader.NameTable'. Considere inspecionar o primeiro argumento: contentReader. ResXResourceReader.cs 267

 private void EnsureResData() { .... XmlTextReader contentReader = null; try { if (fileContents != null) { contentReader = new XmlTextReader(....); } else if (reader != null) { contentReader = new XmlTextReader(....); } else if (fileName != null || stream != null) { .... contentReader = new XmlTextReader(....); } SetupNameTable(contentReader); // <= .... } finally { .... } .... } 

Veja o que acontece com a variável contentReader no corpo do método. Após a inicialização com uma referência nula, como resultado de uma das verificações, o link será inicializado. No entanto, uma série de verificações não termina com um bloco else . Isso significa que, em alguns casos raros (ou devido à refatoração no futuro), o link ainda pode permanecer zero. Em seguida, ele será passado para o método SetupNameTable , onde é usado sem nenhuma verificação:

 private void SetupNameTable(XmlReader reader) { reader.NameTable.Add(ResXResourceWriter.TypeStr); reader.NameTable.Add(ResXResourceWriter.NameStr); .... } 

Este é um código potencialmente inseguro.

E mais um erro, em que o analisador teve que passar por uma cadeia de chamadas para identificar um problema:

PVS-Studio: V3080 Possível desreferência nula. Considere inspecionar o 'layout'. DockAndAnchorLayout.cs 156

 private static Rectangle GetAnchorDestination( IArrangedElement element, Rectangle displayRect, bool measureOnly) { .... AnchorInfo layout = GetAnchorInfo(element); int left = layout.Left + displayRect.X; .... } 

O analisador afirma que é possível obter uma referência nula do método GetAnchorInfo , que emitirá uma exceção ao calcular o valor esquerdo . Vamos percorrer toda a cadeia de chamadas e verificar se é assim:

 private static AnchorInfo GetAnchorInfo(IArrangedElement element) { return (AnchorInfo)element.Properties.GetObject(s_layoutInfoProperty); } public object GetObject(int key) => GetObject(key, out _); public object GetObject(int key, out bool found) { short keyIndex = SplitKey(key, out short element); if (!LocateObjectEntry(keyIndex, out int index)) { found = false; return null; } // We have found the relevant entry. See if // the bitmask indicates the value is used. if (((1 << element) & s_objEntries[index].Mask) == 0) { found = false; return null; } found = true; switch (element) { case 0: return s_objEntries[index].Value1; .... default: Debug.Fail("Invalid element obtained from LocateObjectEntry"); return null; } } 

De fato, em alguns casos, o método GetObject que fecha a cadeia de chamadas retornará nulo , que sem nenhuma verificação adicional será passado para o método de chamada. Provavelmente, o método GetAnchorDestination deve prever essa situação.

No código WinForms, havia alguns desses erros, mais de 70 . Eles são todos semelhantes e eu não darei sua descrição no artigo.

PVS-Studio: V3091 Análise empírica. É possível que um erro de digitação esteja presente dentro da cadeia literal: "ShowCheckMargin". A palavra 'ShowCheckMargin' é suspeita. PropertyNames.cs 136

 internal class PropertyNames { .... public static readonly string ShowImageMargin = "ShowCheckMargin"; ... public static readonly string ShowCheckMargin = "ShowCheckMargin"; .... } 

Um bom exemplo de erro que não é tão fácil de detectar. Ao inicializar os campos de classe, eles usam o mesmo valor, embora o autor do código obviamente não tenha pensado (culpar copiar e colar). O analisador chegou a essa conclusão comparando os nomes das variáveis ​​e os valores das seqüências atribuídas. Eu dei apenas as linhas de erro, mas veja como fica no editor de código:

Quadro 2

É a detecção de tais erros que demonstra o poder e o cuidado infinito das ferramentas de análise estática.

PVS-Studio: V3095 O objeto 'currentForm' foi usado antes de ser verificado com valor nulo. Verifique as linhas: 3386, 3404. Application.cs 3386

 private void RunMessageLoopInner(int reason, ApplicationContext context) { .... hwndOwner = new HandleRef( null, UnsafeNativeMethods.GetWindowLong( new HandleRef(currentForm, currentForm.Handle), // <= NativeMethods.GWL_HWNDPARENT)); .... if (currentForm != null && ....) .... } 

Clássico A variável currentForm é usada sem nenhuma verificação. Mas ainda mais no código, verifica-se a igualdade nula . Nesse caso, posso aconselhá-lo a ter mais cuidado ao trabalhar com tipos de referência, além de usar analisadores estáticos :).

Outro erro semelhante:

PVS-Studio: V3095 O objeto 'backgroundBrush' foi usado antes de ser verificado com valor nulo. Verifique as linhas: 2331, 2334. DataGrid.cs 2331

 public Color BackgroundColor { .... set { .... if (!value.Equals(backgroundBrush.Color)) // <= { if (backgroundBrush != null && BackgroundBrush != DefaultBackgroundBrush) .... } } } 

No código WinForms, encontrei mais de 60 desses erros. Na minha opinião, todos eles são bastante críticos e requerem a atenção dos desenvolvedores. Mas no artigo não é tão interessante falar sobre eles, então vou me limitar aos dois discutidos acima.

PVS-Studio: V3125 O objeto '_propInfo' foi usado e verificado contra nulo em diferentes ramificações de execução. Verifique as linhas: 996, 982. Binding.cs 996

 private void SetPropValue(object value) { .... if (....) { if .... else if (_propInfo != null) .... } else { _propInfo.SetValue(_control, value); } .... } 

Para completar a imagem - também uma espécie de clássico, bug V3125 . A situação inversa. No início, uma referência potencialmente nula é usada com segurança, verificando se é nula , mas elas não fazem mais isso no código.

E outro erro semelhante:

PVS-Studio: V3125 O objeto 'proprietário' foi usado depois que foi verificado contra nulo. Verifique as linhas: 64, 60. FlatButtonAppearance.cs 64

 public int BorderSize { .... set { .... if (owner != null && owner.ParentInternal != null) { LayoutTransaction.DoLayoutIf(....); } owner.Invalidate(); // <= .... } } 

Beleza Mas isso é do ponto de vista de um pesquisador externo. De fato, além desses dois V3125s , o analisador encontrou mais de 50 padrões semelhantes no código WinForms. Os desenvolvedores têm trabalho a fazer.

E finalmente - um erro bastante interessante, na minha opinião.

PVS-Studio: V3137 A variável 'hCurrentFont' é atribuída, mas não é usada no final da função. DeviceContext2.cs 241

 sealed partial class DeviceContext : .... { WindowsFont selectedFont; .... internal void DisposeFont(bool disposing) { if (disposing) { DeviceContexts.RemoveDeviceContext(this); } if (selectedFont != null && selectedFont.Hfont != IntPtr.Zero) { IntPtr hCurrentFont = IntUnsafeNativeMethods.GetCurrentObject( new HandleRef(this, hDC), IntNativeMethods.OBJ_FONT); if (hCurrentFont == selectedFont.Hfont) { // select initial font back in IntUnsafeNativeMethods.SelectObject(new HandleRef(this, Hdc), new HandleRef(null, hInitialFont)); hCurrentFont = hInitialFont; // <= } selectedFont.Dispose(disposing); selectedFont = null; } } .... } 

Vamos ver o que o analisador alertou e por que o fato de uma variável receber um valor, mas não ser usado mais no método, pode indicar um problema.

Uma classe parcial é declarada no arquivo DeviceContext2.cs . O método DisposeFont é usado para liberar recursos depois de trabalhar com gráficos: contexto do dispositivo e fontes. Para uma melhor compreensão, forneço o método DisposeFont inteiro. Preste atenção na variável local hCurrentFont . O problema é que declarar essa variável em um método oculta o campo de classe com o mesmo nome. Encontrei dois métodos da classe DeviceContext em que um campo chamado hCurrentFont é usado :

 public IntPtr SelectFont(WindowsFont font) { .... hCurrentFont = font.Hfont; .... } public void ResetFont() { .... hCurrentFont = hInitialFont; } 

Dê uma olhada no método ResetFont . A última linha é exatamente o que o método DisposeFont faz no bloco aninhado se (o analisador aponta para esse local). E esse campo hCurrentFont com o mesmo nome foi declarado em outra parte da classe parcial no arquivo DeviceContext.cs :

 sealed partial class DeviceContext : .... { .... IntPtr hInitialFont; .... IntPtr hCurrentFont; // <= .... } 

Assim, um erro óbvio foi cometido. Outra questão é a sua criticidade. Agora, como resultado do método DisposeFont trabalhando na seção marcada com o comentário “selecione a fonte inicial de volta”, o campo hCurrentFont não receberá um valor inicial. Eu acho que apenas os autores do código podem dar um veredicto exato.

Conclusões

Então, desta vez eu tenho que "repreender" a MS um pouco. No WinForms, houve muitos erros que exigem muita atenção dos desenvolvedores. Talvez isso se deva a alguma pressa com a qual a MS está trabalhando no .NET Core 3 e componentes, incluindo o WinForms. Na minha opinião, o código WinForms ainda está "úmido", mas espero que a situação mude em breve para melhor.

A segunda razão para um grande número de erros pode ser que nosso analisador ficou melhor ao procurar por eles :).

A propósito, um artigo do meu colega Sergey Vasiliev será publicado em breve, no qual ele pesquisará e encontrará alguns problemas no código das bibliotecas do .NET Core. Espero que o trabalho dele também contribua para melhorar o desempenho da plataforma .NET, porque sempre tentamos comunicar os resultados da análise de seus projetos aos desenvolvedores.

Bem, para aqueles que desejam melhorar seus produtos por conta própria ou realizar pesquisas para encontrar erros nos projetos de outras pessoas, sugiro fazer o download e experimentar o PVS-Studio.

Todo o código limpo!


Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Sergey Khrenov. WinForms: erros, Holmes

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


All Articles