winforms教程_WinForms:错误,福尔摩斯

winforms教程

Picture 5

We like to search for errors in Microsoft projects. Why? It's simple: their projects are usually easy to check (you can work in Visual Studio environment for which PVS-Studio has a convenient plugin) and they contain few errors. That's why the usual work algorithm is as follows: find and download an open source project from MS; check it; choose interesting errors; make sure there are few of them; write an article without forgetting to praise the developers. Great! Win-win-win: it took a little time, the bosses are glad to see new materials in the blog, and karma is fine. But this time «something went wrong». Let's see what we have found in the source code of Windows Forms and whether we should speak highly of Microsoft this time.

我们喜欢在Microsoft项目中搜索错误。 为什么? 很简单:它们的项目通常很容易检查(您可以在PVS-Studio具有便捷插件的Visual Studio环境中工作),并且它们包含的错误很少。 这就是为什么通常的工作算法如下:从MS查找并下载一个开源项目; 核实; 选择有趣的错误; 确保其中很少; 写一篇文章,不要忘记赞扬开发人员。 大! 双赢:花了一些时间,老板很高兴在博客中看到新资料,业力很好。 但是这次“出事了”。 让我们看看我们在Windows Forms的源代码中发现了什么,以及我们这次是否应该高度评价Microsoft。

介绍 (Introduction)

In early December 2018, Microsoft announced the release of the .NET Core 3 Preview 1. A little earlier (about mid-October), GitHub started to actively disclose the sources of Windows Forms — the .NET Core UI platform for creating Windows desktop applications. You can see the commit statistics here. Now anyone can download the WinForms source code for review.

在2018年12月上旬,微软宣布发布.NET Core 3 Preview 1.在不久之前(大约10月中旬),GitHub开始积极披露Windows Forms的来源-用于创建Windows桌面应用程序的.NET Core UI平台。 。 您可以在此处查看提交统计信息。 现在,任何人都可以下载WinForms源代码以进行审查。

I also downloaded the sources to search for errors there with PVS-Studio. The check did not cause any difficulties. We needed: Visual Studio 2019, .NET Core 3.0 SDK Preview, PVS-Studio. And here we have the log of the analyzer's warnings.

我还下载了源代码,以使用PVS-Studio搜索错误。 检查没有造成任何困难。 我们需要:Visual Studio 2019,.NET Core 3.0 SDK预览,PVS-Studio。 这里有分析仪警告的日志。

Having received the PVS-Studio report, I usually sort it by diagnostic numbers in the ascending order (the window with the PVS-Studio message log in Visual Studio environment has various options of sorting and filtering the list). It allows you to work with groups of similar errors, which greatly simplifies source code analysis. I mark interesting errors in the list with a «star» and only then, after analyzing the whole log, I write out code fragments and describe them. Since there are usually few errors, I «stir» them trying to place the most interesting ones at the beginning and end of the article. But this time it turned out to be a lot of errors (eh, the intrigue has not been saved for a long time) and I will cite them in the order of numbers of diagnostics.

收到PVS-Studio报告后,我通常按诊断编号以升序对其进行排序(Visual Studio环境中带有PVS-Studio消息日志的窗口具有各种排序和过滤列表的选项)。 它使您可以处理类似错误的组,从而大大简化了源代码分析。 我在列表中用“星号”标记了有趣的错误,然后,在分析了整个日志之后,我写出了代码片段并进行了描述。 由于通常很少有错误,因此我“搅拌”它们,试图将最有趣的错误放在文章的开头和结尾。 但是这次事实证明是很多错误(嗯,很长一段时间以来都没有保存这种阴谋),我将按照诊断次数的顺序来引用它们。

What did we find? 833 High and Medium warnings (249 and 584, respectively) were issued for 540,000 lines of code (not including empty ones) in 1670 cs files. And yes, traditionally I didn't check the tests and didn't consider the Low warnings (there were 215 of them). According to my previous observations, the warnings are too many for the MS project. But not all the warnings are errors.

我们发现了什么? 在1670 cs文件中,对540,000行代码(不包括空代码)发出了833个高警告和中警告(分别为249和584)。 是的,传统上我不检查测试,也不考虑“低”警告(其中有215个)。 根据我以前的观察,对于MS项目,警告太多了。 但并非所有警告都是错误。

For this project the number of false alarms was about 30%. In about 20% of cases, I just could not make an exact conclusion whether it was an error or not because I was not familiar with the code well enough. And at least 20% of the errors I missed can be written off as «human factor»: haste, tiredness, etc. By the way, the opposite effect is also possible: some same-type triggers, the number of which could reach 70-80, I looked «next but one», which sometimes could increase the number of errors that I thought were real.

对于该项目,错误警报的数量约为30%。 在大约20%的情况下,因为我对代码不够熟悉,所以我无法确切地得出结论是否是错误。 而且我错过的错误中至少有20%可以写为“人为因素”:仓促,疲倦等。顺便说一句,相反的效果也是可能的:某些相同类型的触发器,其数量可能达到70 -80,我看上去“仅此而已”,有时这会增加我认为是真实的错误的数量。

Anyway, 30% of the warnings indicate real errors, which is quite a large percentage if you take into account that the analyzer was not pre-configured.

无论如何,30%的警告表示实际错误,如果考虑到分析仪未预先配置,则这是一个很大的百分比。

So, the number of errors I managed to find was about 240, which is within the range of the given statistics. Again, in my opinion, this is not the most outstanding result for a MS project (although it will make only 0.44 errors per 1000 code lines) and there are probably more real errors in WinForms code as well. I suggest considering the reasons at the end of the article and now let's see the most interesting errors.

因此,我设法找到的错误数约为240,这在给定的统计范围内。 同样,在我看来,这对于MS项目而言并不是最出色的结果(尽管每1000条代码行只会产生0.44错误),而且WinForms代码中也可能存在更多实际错误。 我建议在文章末尾考虑原因,现在让我们看看最有趣的错误。

失误 (Errors)

PVS-Studio: V3003 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 213, 224. ButtonStandardAdapter.cs 213

PVS-Studio: V3003已检测到'if(A){...} else if(A){...}'模式的使用。 存在逻辑错误的可能性。 检查行: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
  {
    ....
  }
  ....
}
If and ifelse if blocks check the same condition. It looks like copy-paste. Is it an error? If you look at the declaration of the else if块检查相同的条件。 看起来像是复制粘贴。 这是错误吗? 如果查看 IsHighContrastHighlighted method, you may doubt it: IsHighContrastHighlighted方法的声明,您可能会怀疑:
protected bool IsHighContrastHighlighted()
{
  return SystemInformation.HighContrast && 
    Application.RenderWithVisualStyles &&
    (Control.Focused || Control.MouseIsOver || 
      (Control.IsDefault && Control.Enabled));
}

The method can probably return different values for sequential calls. And what is happening in the caller method, of course, looks strange, but has the right to exist. However, I would advise the authors to take a look at this code fragment. Just in case. It is also a good example of how difficult it is to draw conclusions when analyzing unfamiliar code.

该方法可能会为顺序调用返回不同的值。 当然,调用方方法中发生的事情看起来很奇怪,但是具有存在的权利。 但是,我建议作者看一下此代码片段。 以防万一。 这也是一个很好的例子,说明在分析不熟悉的代码时很难得出结论。

PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. RichTextBox.cs 1018

PVS-Studio: V3004'then '语句等效于'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;  // <=
    }
    ....
  }
  ....
}

And there is definitely a copy-paste error here. Regardless of the condition, the selCharOffset variable will always get the same value.

而且这里肯定存在复制粘贴错误。 无论条件如何, selCharOffset变量将始终获得相同的值。

There are two more such errors in WinForms code:

WinForms代码中还有两个这样的错误:

  • V3004 The 'then' statement is equivalent to the 'else' statement. SplitContainer.cs 1700

    V3004'then'语句等效于'else'语句。 SplitContainer.cs 1700
  • V3004 The 'then' statement is equivalent to the 'else' statement. ToolstripProfessionalRenderer.cs 371

    V3004'then'语句等效于'else'语句。 工具tripProfessionalRenderer.cs 371

PVS-Studio: V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 681, 680. ProfessionalColorTable.cs 681

PVS-Studio: V3008连续两次为变量分配值。 也许这是一个错误。 检查行:681、680。ProfessionalColorTable.cs 681

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

The method fills the rgbTable dictionary. The analyzer pointed to a code fragment where different values are written twice on the same key in sequence. Things would be fine but there are still 16 such fragments in this method. It does not look like a one-of-a-kind error anymore. But why they do this remains a mystery to me. I didn't find any signs of autogenerated code. It looks like this in the editor:

该方法填充rgbTable字典。 分析器指向一个代码片段,在该片段中,将不同的值依次写入同一键两次。 一切都会好起来的,但是这种方法仍然有16个这样的片段。 它看起来不再像一种错误。 但是为什么他们这样做对我来说仍然是个谜。 我没有发现任何自动生成代码的迹象。 在编辑器中看起来像这样:

Picture 3

I'll give you the first ten warnings on the list:

我将在列表中给您前十个警告:

  1. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 785, 784. ProfessionalColorTable.cs 785

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:785,784。ProfessionalColorTable.cs 785
  2. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 787, 786. ProfessionalColorTable.cs 787

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:787、786。ProfessionalColorTable.cs 787
  3. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 789, 788. ProfessionalColorTable.cs 789

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:789、788。ProfessionalColorTable.cs 789
  4. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 791, 790. ProfessionalColorTable.cs 791

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:791、790。ProfessionalColorTable.cs 791
  5. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 797, 796. ProfessionalColorTable.cs 797

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:797、796。ProfessionalColorTable.cs 797
  6. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 799, 798. ProfessionalColorTable.cs 799

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:799、798。ProfessionalColorTable.cs 799
  7. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 807, 806. ProfessionalColorTable.cs 807

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:807、806。ProfessionalColorTable.cs 807
  8. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 815, 814. ProfessionalColorTable.cs 815

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:815、814。ProfessionalColorTable.cs 815
  9. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 817, 816. ProfessionalColorTable.cs 817

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:817,816。ProfessionalColorTable.cs 817
  10. V3008 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 823, 822. ProfessionalColorTable.cs 823

    V3008连续两次为变量分配值。 也许这是一个错误。 检查行:823,822。ProfessionalColorTable.cs 823

PVS-Studio: V3011 Two opposite conditions were encountered. The second condition is always false. Check lines: 5242, 5240. DataGrid.cs 5242

PVS-Studio: V3011遇到两个相反的条件。 第二个条件始终为假。 检查行: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;  
  }
}

The return operator will never be executed. Most likely, the myGridTable != null condition in the external if block was added later during refactoring. And now the check of myGridTable == null is meaningless. To improve the code quality, you should remove this check.

返回运算符将永远不会执行。 最有可能的是,稍后在重构期间在外部if块中添加了myGridTable!= null条件。 现在, myGridTable == null的检查是没有意义的。 为了提高代码质量,您应该删除此检查。

PVS-Studio: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'left', 'cscLeft'. TypeCodeDomSerializer.cs 611

PVS-Studio: V3019可能在使用'as'关键字进行类型转换后将不正确的变量与null进行比较。 检查变量“ left”,“ cscLeft”。 TypeCodeDomSerializer.cs 611

PVS-Studio: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'right', 'cscRight'. TypeCodeDomSerializer.cs 615

PVS-Studio: V3019可能在使用'as'关键字进行类型转换后将不正确的变量与null进行比较。 检查变量“ 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;  // <=
}

The analyzer generated two warnings for the Compare method at once. What is the problem? It is that cscLeft and cscRight values are not checked for null at all. They may get this value after unsuccessful casting to the OrderedCodeStatementCollection type. Then an exception will be thrown in the last return expression. This situation is possible when all the checks for left and right pass and do not lead to a preliminary exit from the method.

分析器立即为Compare方法生成了两个警告。 问题是什么? 完全不检查cscLeftcscRight值是否为null 。 在未成功转换为OrderedCodeStatementCollection类型后,他们可能会获得此值。 然后,将在最后一个返回表达式中引发异常。 这种情况是可能的,当所有的左侧右侧传中的检查并不会导致从方法的初步退出。

To fix the code, you should use cscLeft/cscRight instead of left/right everywhere.

要修复该代码,您应该使用cscLeft / cscRight而不是在各处使用左/右

PVS-Studio: V3020 An unconditional 'break' within a loop. SelectionService.cs 421

PVS-Studio: V3020循环内无条件的“中断”。 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;
    }
  }
  ....            
}

This fragment refers rather to the «code smell». There is no error here. But questions arise about the way the foreach loop is organized. It is clear why it is needed here: because of the need to extract elements of the collection, passed as ICollection. But why did the loop, initially designed for single iteration (the precondition is the presence of a single element in the collection components), require additional support such as break? Probably, the answer can be considered as follows: «Historically, this has come to be». The code looks ugly.

该片段指的是“代码气味”。 这里没有错误。 但是,人们对foreach循环的组织方式提出了疑问。 很明显为什么在这里需要它:因为需要提取作为ICollection传递的集合的元素。 但是,为什么最初设计用于单次迭代的循环(前提是集合组件中存在单个元素),却需要诸如break之类的额外支持? 答案可能如下:“从历史上讲,这已经成为事实”。 该代码看起来很难看。

PVS-Studio: V3022 Expression 'ocxState != null' is always true. AxHost.cs 2186

PVS-Studio: V3022表达式'ocxState!= null'始终为true。 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;
    } 
    ....
  }
}

Because of a logical error, «dead code» occurred in this fragment. Expressions in the else block will never be executed.

由于逻辑错误,此片段中发生了“死代码”。 else块中的表达式将永远不会执行。

PVS-Studio: V3027 The variable 'e' was utilized in the logical expression before it was verified against null in the same logical expression. ImageEditor.cs 99

PVS-Studio: V3027在针对同一逻辑表达式中的null进行验证之前,在逻辑表达式中使用了变量'e'。 图像编辑器.cs 99

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

Variable e in the condition is first used and then checked against null. Hello, NullReferenceException.

首先使用条件中的变量e ,然后针对null进行检查。 您好, NullReferenceException

One more such error:

还有一个这样的错误:

PVS-Studio: V3027 The variable 'dropDownItem' was utilized in the logical expression before it was verified against null in the same logical expression. ToolStripMenuItemDesigner.cs 1351

PVS-Studio: V3027在对同一逻辑表达式中的null进行验证之前,在逻辑表达式中使用了变量'dropDownItem'。 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)
  {
    ....
  }
  ....
}

The situation is similar to the previous one but with the dropDownItem variable. I think that such errors appear as a result of careless refactoring. Probably, a part of the condition !(dropDownItem.Owner is ToolStripDropDownMenu) was added into the code later.

这种情况与前一种情况相似,但具有dropDownItem变量。 我认为此类错误是由于粗心的重构而出现的。 稍后,部分条件!(dropDownItem.Owner是ToolStripDropDownMenu)可能已添加到代码中。

PVS-Studio: V3030 Recurring check. The 'columnCount > 0' condition was already verified in line 3900. ListView.cs 3903

PVS-Studio: V3030定期检查。 'columnCount> 0'条件已经在行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);
    }
    ....
  }
  ....
}

A mistake that may seem harmless. Indeed, an unnecessary check is performed which does not affect the operational logic. And sometimes it is even done when you need to check the state of some visual component again, for example, getting the number of entries in the list. But in this case the local variable columnCount is checked twice. It is very suspicious. Either they wanted to check another variable or they used a wrong condition in one of the checks.

一个看似无害的错误。 实际上,执行了不会影响操作逻辑的不必要的检查。 有时甚至在您需要再次检查某些视觉组件的状态时也可以完成此操作,例如,获取列表中的条目数。 但是在这种情况下,将对局部变量columnCount进行两次检查。 非常可疑。 他们要么想要检查另一个变量,要么在其中一项检查中使用了错误的条件。

PVS-Studio: V3061 Parameter 'lprcClipRect' is always rewritten in method body before being used. WebBrowserSiteBase.cs 281

PVS-Studio: V3061参数“ lprcClipRect”在使用前总是在方法体中重写。 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;
}

An unevident mistake. Yes, the lprcClipRect parameter is actually initialized with a new value without using it in any way. But what does it lead to in the end? I think that somewhere in the calling code the reference passed through this parameter will remain unchanged, although it was not intended to be so. Really, appreciate the handling of other variables in this method. Even its name («Get» prefix) hints that some initialization will be performed inside the method through passed parameters. And it is so. The first two parameters (ppFrame and ppDoc) are passed with the out modifier and they get new values. References lprcPosRect and lpFrameInfo are used to access and initialize class fields. Only lprcClipRect stands out. Probably, the out or ref modifier is required for this parameter.

一个不明显的错误。 是的, lprcClipRect参数实际上是使用新值初始化的,而不以任何方式使用它。 但是最终会导致什么呢? 我认为在调用代码中的某个地方,通过此参数传递的引用将保持不变,尽管并非如此。 确实,请欣赏此方法中其他变量的处理。 即使它的名称(“ Get”前缀)也暗示将通过传递的参数在方法内部执行一些初始化。 就是这样。 前两个参数( ppFrameppDoc )与out修饰符一起传递,并且它们获得新值。 引用lprcPosRectlpFrameInfo用于访问和初始化类字段。 只有lprcClipRect脱颖而出。 此参数可能需要outref修饰符。

PVS-Studio: V3066 Possible incorrect order of arguments passed to 'AdjustCellBorderStyle' method: 'isFirstDisplayedRow' and 'isFirstDisplayedColumn'. DataGridViewComboBoxCell.cs 1934

PVS-Studio: V3066传递给“ AdjustCellBorderStyle”方法的参数的可能错误顺序:“ isFirstDisplayedRow”和“ isFirstDisplayedColumn”。 DataGridViewComboBoxCell.cs 1934

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

The analyzer suspected that the last two arguments were mixed up. Let's take a look at the declaration of the AdjustCellBorderStyle method:

分析器怀疑最后两个参数混淆了。 让我们看一下AdjustCellBorderStyle方法的声明:

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

Looks like a mistake. Yes, some arguments are often passed in reverse order, for example, to exchange some variables. But I don't think this is the case. Nothing in the caller or callee methods indicates this usage pattern. First, variables of the bool type are mixed up. Second, the names of the methods are also regular: no «Swap» or «Reverse». Besides, it is not so difficult to make a mistake like that. People often perceive the order of the «row/column» pair differently. For me, for example, it is the «row/column» that is familiar. But for the author of the method called AdjustCellBorderStyle, obviously, the more usual order is «column/row».

看起来像是一个错误。 是的,某些参数通常以相反的顺序传递,例如交换某些变量。 但我认为情况并非如此。 调用方或被调用方方法中的任何内容均未指示此使用模式。 首先,将布尔类型的变量混合在一起。 其次,方法的名称也是常规的:没有“交换”或“反向”。 此外,犯这样的错误并不难。 人们通常对“行/列”对的顺序有不同的认识。 例如,对我而言,熟悉的是“行/列”。 但是对于名为AdjustCellBorderStyle的方法的作者来说 ,显然,更常见的顺序是“列/行”。

PVS-Studio: V3070 Uninitialized variable 'LANG_USER_DEFAULT' is used when initializing the 'LOCALE_USER_DEFAULT' variable. NativeMethods.cs 890

PVS-Studio: V3070初始化“ LOCALE_USER_DEFAULT”变量时使用未初始化的变量“ LANG_USER_DEFAULT”。 本地方法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);
  ....
}

Rare mistake. The initialization order of class fields is mixed up. To calculate the value of the field LOCALE_USER_DEFAULT the LANG_USER_DEFAULT field is used, which is not yet initialized and has a value of 0. By the way, the LANG_USER_DEFAULT variable is not used anywhere else in the code. I went an extra mile and wrote a small console program that simulates the situation. I substituted some constants used in WinForms code with their actual values:

罕见的错误。 类字段的初始化顺序混合在一起。 要计算字段LOCALE_USER_DEFAULT的值,使用LANG_USER_DEFAULT字段,该字段尚未初始化且值为0。顺便说一下,在代码的其他任何地方都没有使用LANG_USER_DEFAULT变量。 我付出了更多努力,并编写了一个小型控制台程序来模拟这种情况。 我将WinForms代码中使用的一些常量替换为其实际值:

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);
  }
}

As a result, the console will display: 0. Now let's swap the declarations of the LOCALE_USER_DEFAULT and LANG_USER_DEFAULT fields. The result of the program execution is as follows: 1024. I think there is nothing more to comment on here.

结果,控制台将显示:0。现在,我们交换LOCALE_USER_DEFAULTLANG_USER_DEFAULT字段的声明。 程序执行的结果如下:1024。我认为这里没有更多评论。

PVS-Studio: V3080 Possible null dereference. Consider inspecting 'ces'. CodeDomSerializerBase.cs 562

PVS-Studio: V3080可能取消空引用。 考虑检查“ ces”。 第562章

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

The code that should «crash» rather regularly, because you can get into the else branch just when the ces reference equals null.

该代码应该定期“崩溃”,因为仅当ces引用等于null时,您才能进入else分支。

Another similar example:

另一个类似的例子:

PVS-Studio: V3080 Possible null dereference. Consider inspecting 'comboBox'. ComboBox.cs 6610

PVS-Studio: V3080可能取消空引用。 考虑检查“ comboBox”。 ComboBox.cs 6610

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

The paradoxical code. Apparently, the if (comboBox != null) check was confused withif (comboBox == null). And so, we will get another NullReferenceException.

矛盾的代码。 显然, if(comboBox!= null)检查与if(comboBox == null)混淆了。 因此,我们将获得另一个NullReferenceException。

We have considered two rather obvious V3080 errors where you can visually trace a potential null reference usage within a method. But the V3080 diagnostic is much more efficient and can find such errors for method call chains. Not so long ago we have significantly improved the dataflow and interprocedural analysis mechanisms. You may read about this in the article "Nullable Reference types in C# 8.0 and static analysis". But here is such kind of error detected in WinForms:

我们考虑了两个非常明显的V3080错误,您可以在其中直观地跟踪方法中潜在的空引用用法。 但是V3080诊断效率更高,并且可以为方法调用链找到此类错误。 不久前,我们已经大大改善了数据流和过程间分析机制。 您可以在文章“ C#8.0和静态分析中的可空引用类型 ”中阅读有关此内容的信息。 但是,这是在WinForms中检测到的此类错误:

PVS-Studio: V3080 Possible null dereference inside method at 'reader.NameTable'. Consider inspecting the 1st argument: contentReader. ResXResourceReader.cs 267

PVS-Studio: V3080'reader.NameTable '中的方法内部可能存在空取消引用。 考虑检查第一个参数: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
  {
    ....
  }
  ....
}

Look what happens to the contentReader variable in the method body. After initialization with null, it will be initialized again in one of the checks. But the series of checks does not end with the else block. It means that in some rare case (or due to refactoring in the future) the reference

查看方法主体中的contentReader变量发生了什么。 使用null初始化后,将在其中一项检查中再次对其进行初始化。 但是一系列的检查并没有以else块结尾。 这意味着在极少数情况下(或由于将来的重构),参考

威力 (might )

still remain null. Then it will be passed to the

仍然为空。 然后将其传递给

不使用任何检查的 SetupNameTable method where it is used without any check: SetupNameTable方法:
private void SetupNameTable(XmlReader reader)
{
  reader.NameTable.Add(ResXResourceWriter.TypeStr);
  reader.NameTable.Add(ResXResourceWriter.NameStr);
  ....
}

This is potentially unsafe code.

这可能是不安全的代码。

And one more error where the analyzer had to go through the call chain to detect the problem:

分析器不得不通过调用链来检测问题的又一个错误:

PVS-Studio: V3080 Possible null dereference. Consider inspecting 'layout'. DockAndAnchorLayout.cs 156

PVS-Studio: V3080可能取消空引用。 考虑检查“布局”。 第156章

private static Rectangle GetAnchorDestination(
  IArrangedElement element, Rectangle displayRect, bool measureOnly)
{
  ....
  AnchorInfo layout = GetAnchorInfo(element);

  int left = layout.Left + displayRect.X;
  ....
}

The analyzer claims that it is possible to get a null reference from the GetAnchorInfo method, which will cause an exception when calculating the left value. Let's go through the whole call chain and check if it is true:

分析器声称可以从GetAnchorInfo方法获取空引用,这将在计算值时引起异常。 让我们遍历整个调用链,并检查其是否正确:

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;
  }
}

Indeed, in some cases, the GetObject method that ends the call chain will return null, which will be passed to the caller method without any additional checks. Probably, it is necessary to cover such a situation in the GetAnchorDestination method.

实际上,在某些情况下,结束调用链的GetObject方法将返回null ,该值将传递给调用方方法而无需任何其他检查。 可能有必要在GetAnchorDestination方法中解决这种情况。

There are quite a lot of such errors in WinForms code,

WinForms代码中有很多此类错误,

超过70 (more than 70)

. They all look alike and I will not describe them in the article.

。 它们看起来都很相似,我将不在本文中描述它们。

PVS-Studio: V3091 Empirical analysis. It is possible that a typo is present inside the string literal: «ShowCheckMargin». The 'ShowCheckMargin' word is suspicious. PropertyNames.cs 136

PVS-Studio: V3091实证分析。 字符串文字“«ShowCheckMargin»”内可能存在错字。 “ ShowCheckMargin”一词可疑。 PropertyNames.cs 136

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

A good example of an error that is not so easy to find. When initializing the class fields the same value is used although the author of the code obviously did not intend so (copy-paste is to blame). The analyzer made this conclusion by comparing the names of variables and values of assigned strings. I have given only lines with errors but you should check it out how it looks in the code editor:

一个不那么容易发现的错误的好例子。 初始化类字段时,使用相同的值,尽管代码的作者显然不打算这样做(应归咎于复制粘贴)。 分析器通过比较变量名称和分配的字符串的值来得出此结论。 我只给出了有错误的行,但是您应该在代码编辑器中检查一下它的外观:

Picture 2

Detection of such errors is what demonstrates all the power and endless attention span of static analysis tools.

对此类错误的检测证明了静态分析工具的强大功能和无尽的关注范围。

PVS-Studio: V3095 The 'currentForm' object was used before it was verified against null. Check lines: 3386, 3404. Application.cs 3386

PVS-Studio: V3095在对null进行验证之前,已使用'currentForm'对象。 检查行: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 && ....)
  ....
}

This is classic. The currentForm variable is used without any checks. But then it's checked for null in the code. In this case I can advise you to be more attentive when working with reference types and also use static analyzers :).

这是经典。 使用currentForm变量无需任何检查。 但是,然后在代码中检查是否为 。 在这种情况下,我建议您在使用引用类型时也要多加注意,并使用静态分析器:)。

One more such error:

还有一个这样的错误:

PVS-Studio: V3095 The 'backgroundBrush' object was used before it was verified against null. Check lines: 2331, 2334. DataGrid.cs 2331

PVS-Studio: V3095在对null进行验证之前,已使用“ backgroundBrush”对象。 检查行:2331,2334。DataGrid.cs 2331

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

In WinForms code, I came across

在WinForms代码中,我遇到了

超过60 (more than 60)

such errors. In my opinion, all of them are rather critical and require attention of developers. But it is not so interesting to tell about them in the article anymore, so I will limit myself to the two mentioned above.

这样的错误。 我认为,所有这些都非常关键,需要开发人员的注意。 但是在文章中再介绍它们并没有那么有趣,所以我将限于上述两个。

PVS-Studio: V3125 The '_propInfo' object was used and was verified against null in different execution branches. Check lines: 996, 982. Binding.cs 996

PVS-Studio: V3125已使用'_propInfo'对象,并在不同的执行分支中针对null对其进行了验证。 检查行:996、982。Binding.cs 996

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

For the completeness sake — also a kind of classic, error V3125. The opposite situation. At first, the developer uses a potentially null reference safely, having checked it against null, but stops doing it further in the code.

为了完整起见,这也是一种经典的错误V3125 。 相反的情况。 首先,开发人员已将其与null进行检查,然后安全地使用了可能为null的引用,但在代码中不再做进一步的操作。

And one more such error:

还有一个这样的错误:

PVS-Studio: V3125 The 'owner' object was used after it was verified against null. Check lines: 64, 60. FlatButtonAppearance.cs 64

PVS-Studio: V3125在针对null进行验证之后,使用了“所有者”对象。 检查行:64,60。FlatButtonAppearance.cs 64

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

Lovely. But this an outside researcher's standpoint. After all, the analyzer found

可爱。 但这是外部研究人员的观点。 毕竟,分析仪发现

超过50 (more than 50)

such patterns in WinForms code besides these two

除了这两个以外,WinForms代码中的这种模式

V3125. Developers have a lot to work on. V3125 。 开发人员还有很多工作要做。

And finally, there is an interesting error, in my opinion.

最后,我认为这是一个有趣的错误。

PVS-Studio: V3137 The 'hCurrentFont' variable is assigned but is not used by the end of the function. DeviceContext2.cs 241

PVS-Studio: V3137分配了'hCurrentFont'变量,但在函数结束时未使用该变量。 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;
    }
  }
  ....
}

Let's see what alerted the analyzer, and why it may indicate a problem that a variable is assigned a value, but never used in the code.

让我们看看是什么使分析仪发出了警报,以及为什么它可能指示出一个变量已分配值但在代码中从未使用过的问题。

The DeviceContext2.cs file contains a partial class. The DisposeFont method is used to free resources after working with graphics: device context and fonts. For a better understanding I have given the whole DisposeFont method. Pay attention to the local variable hCurrentFont. The problem is that the declaration of this variable in the method hides the class field of the same name. I found two methods of the DeviceContext class where the

DeviceContext2.cs文件包含部分类。 DisposeFont方法用于处理图形后释放资源:设备上下文和字体。 为了更好地理解,我给出了整个DisposeFont方法。 注意局部变量hCurrentFont 。 问题在于方法中此变量的声明隐藏了相同名称的类字段。 我发现DeviceContext类的两个方法

领域 (field)

with the name

名称

hCurrentFont is used: 使用hCurrentFont
public IntPtr SelectFont(WindowsFont font)
{
  ....
  hCurrentFont = font.Hfont;
  ....
}
public void ResetFont()
{
  ....
  hCurrentFont = hInitialFont;
}

Look at the ResetFont method. The last line there is exactly what the DisposeFont method does in the subblock if (this is what the analyzer points to). This hCurrentFont field of the same name is declared in another part of the partial class in the DeviceContext.cs file:

查看ResetFont方法。 最后一行出现正是DisposeFont方法做的子块,如果 (这是什么分析点)。 在DeviceContext.cs文件的部分类的另一部分中声明了这个具有相同名称的hCurrentFont字段:

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

Thus, an obvious mistake was made. Another question is in its importance. Now, as a result of the DisposeFont method's work in the section marked with the comment «select initial font back in», the hCurrentFont field will not be initialized. I think only the authors of the code can give an exact verdict.

因此,犯了一个明显的错误。 另一个问题是它的重要性。 现在,由于DisposeFont方法在标有“选择初始字体返回”注释的部分中的工作,因此hCurrentFont字段将不会初始化。 我认为只有代码的作者才能给出确切的结论。

结论 (Conclusions)

So, this time, I'm gonna have to criticize MS a little bit. In WinForms, there are a lot of errors that require close attention of developers. Perhaps it is the fault of some haste with which MS work on .NET Core 3 and components, including WinForms. In my opinion, the WinForms code is still «raw», but I hope that the situation will change for the better soon.

所以,这次,我不得不批评一下MS。 在WinForms中,存在许多错误,需要开发人员密切注意。 MS在.NET Core 3和组件(包括WinForms)上使用某些匆忙程序的错误也许是错误的。 我认为WinForms代码仍然很“原始”,但是我希望情况会尽快好转。

The second reason for the large number of errors may be that our analyzer has simply become better at searching for them :).

大量错误的第二个原因可能是我们的分析仪在搜索它们方面变得更加出色:)。

By the way, an article of my colleague Sergey Vasiliev will soon be published in which he searches and finds quite a lot of problems in the code of .NET Core libraries. I hope that his work will also contribute to improving the characteristics of the .NET platform, because we always try to inform the developers about the results of their projects' analysis.

顺便说一下,我的同事Sergey Vasiliev的一篇文章很快就会发表,他在其中搜索并发现.NET Core库代码中的很多问题。 我希望他的工作也将为改善.NET平台的特性做出贡献,因为我们始终尝试将其项目分析结果告知开发人员。

And for those who want to improve their products on their own or search for errors in other people's projects, I suggest that you download and try PVS-Studio.

对于那些想要自己改进产品或在其他人的项目中查找错误的人,建议您下载并尝试PVS-Studio

Clean code to everyone!

干净的代码给大家!

翻译自: https://habr.com/en/company/pvs-studio/blog/462809/

winforms教程

  • 0
    点赞
  • 0
    收藏
    觉得还不错? 一键收藏
  • 0
    评论

“相关推荐”对你有帮助么?

  • 非常没帮助
  • 没帮助
  • 一般
  • 有帮助
  • 非常有帮助
提交
评论
添加红包

请填写红包祝福语或标题

红包个数最小为10个

红包金额最低5元

当前余额3.43前往充值 >
需支付:10.00
成就一亿技术人!
领取后你会自动成为博主和红包主的粉丝 规则
hope_wisdom
发出的红包
实付
使用余额支付
点击重新获取
扫码支付
钱包余额 0

抵扣说明:

1.余额是钱包充值的虚拟货币,按照1:1的比例进行支付金额的抵扣。
2.余额无法直接购买下载,可以购买VIP、付费专栏及课程。

余额充值