PVS-Studio团队即将取得技术突破,但现在让我们重新检查Blender

定期进行静态分析最有用。 尤其是在项目发展迅速时,例如Blender项目。 现在是时候再检查一次,看看这次我们会发现什么可疑片段。

介绍

Blender是一个免费的开源专业3D创作套件。 它支持整个3D管道-建模,装配,动画,模拟,渲染,合成和运动跟踪; 甚至视频编辑和游戏创作。

我们之前已经检查过该项目。 您可以在文章“使用PVS-Studio分析Blender项目”中找到先前对v2.62进行检查的结果。

自上次检查以来,源代码的大小(包括其他库)已增加到77 mb。 现在其代码库为2206 KLOC。 在上次检查时,该项目为68 mb(2105 KLOC)。

SourceMonitorutility对我评估代码库大小很有帮助。 该实用程序能够分析C ++,C,C#,VB.NET,Java和Delphi中的代码,并能够评估各种指标。 例如,它可以确定项目的圈复杂度,还可以为每个项目文件生成详细的统计信息,并将结果显示为表格或图表。

因此,本文是关于Blender v2.77a中发现的错误和可疑片段的。 为了进行分析,我们使用了PVS-Studio 6.05

错别字

在积极使用复制机制和自动完成代码的过程中,各种变量和常量的名称可能会出错。 此类错误可能导致错误的评估结果或意外的程序行为。 在Blender项目中,有几个这样的例子。 让我们仔细看看。

情况错字

  CurvePoint :: CurvePoint(CurvePoint * iA,CurvePoint * iB,float t3) 
{
....
如果((iA-> getPoint2D()-// <=
iA-> getPoint2D())。norm()<1.0e-6){// <=
....
}
....
}

V501′-‘运算符的左侧和右侧有相同的子表达式:iA-> getPoint2D()-iA-> getPoint2D()curve.cpp 136

CurvePoint函数内部,程序将处理两个名称相似的对象-iAiB。 在相当长的条件树中,这些对象的不同方法总是在各种操作中相交。 这些条件块之一中有错字。 结果,我们在一个对象和同一对象的属性之间进行了减法运算。 不知道代码的特殊性,很难说哪个操作数有错误。 我可以提出两种解决方法:

 如果((iA-> getPoint2D()-iB-> getPoint2D())。norm()<1.0e-6).... 

要么

 如果((iB-> getPoint2D()-iA-> getPoint2D())。norm()<1.0e-6).... 

以下错误也隐藏在条件语句中。

  template  
无效JacobiSVD :: allocate(....)
{
....
if(m_cols> m_rows)m_qr_precond_morecols.allocate(* this);
if(m_rows> m_cols)m_qr_precond_morerows.allocate(* this);
if(m_cols!= m_cols)m_scaledMatrix.resize(rows,cols); // <=
}

V501’!=’运算符左右两侧有相同的子表达式:m_cols!= m_cols jacobisvd.h 819

在给定的片段中,您可以看到某个矩阵内的行数和列数相等。 如果数量不相同,则程序将为新元素分配内存并创建它们。 以后,如果添加了新的单元格,则可以更改矩阵大小。 不幸的是,由于条件语句中的错误,将永远不会执行该操作,因为条件m_cols!= m_cols始终为false。 在这种情况下,更改哪个部分并不重要,因此我建议使用以下变体:

  if(m_cols!= m_rows)m_scaledMatrix.resize(行,cols) 

V501诊断程序还检测到更多问题区域:

  • V501’==’运算符的左侧和右侧有相同的子表达式:left.rows()== left.rows()numeric.cc 112
  • V501′>’运算符的左侧和右侧有相同的子表达式:(from [0] [3])>(from [0] [3])stereoimbuf.c 120
  • V501′>’运算符的左侧和右侧有相同的子表达式:(from [0] [3])>(from [0] [3])stereoimbuf.c 157
  • V501在’==’运算符的左侧和右侧有相同的子表达式:out-> y == out-> y filter.c 209

空指针处理

名称中的错字有更严重的后果。

  int QuantitativeInvisibilityF1D :: operator()(....) 
{
ViewEdge * ve = dynamic_cast (&inter);
如果(ve){
结果= ve-> qi();
返回0;
}
FEdge * fe = dynamic_cast (&inter);
如果(fe){
结果= ve-> qi(); // <=
返回0;
}
....
}

V522可能会取消引用空指针“ ve”。 functions1d.cpp 107

这个功能相当短,但是即使是简单的功能,错别字也会使我们陷入困境。 我们可以在代码中看到创建并检查了两个对象。 但是,在检查了第二个对象之后,发生了一个错误,并且即使成功创建了fe ,也不会代替它,而是将第一个对象的功能工作结果写入结果; 根据先前的条件,完全没有创建该对象。 如果更高级别的处理程序未捕获此异常,则很可能导致程序崩溃。

显然,第二个代码片段是使用Copy-Paste编写的。 偶然地,程序员忘记了更改变量名ve。 正确的代码应该像这样:

  FEdge * fe = dynamic_cast (&inter); 
如果(fe){
结果= fe-> qi();
返回0;
}

空指针用法

 静态ImBuf * accessor_get_ibuf(....) 
{
ImBuf * ibuf,* orig_ibuf,* final_ibuf;
....
/ *首先尝试从缓存中获取经过完全处理的图像。 * /
ibuf = accesscache_get(accessor,
clip_index,
帧,
输入模式,
缩小规模
transform_key);
如果(ibuf!= NULL){
返回ibuf;
}
/ *现在我们对原始框架进行后期处理。 * /
orig_ibuf = accessor_get_preprocessed_ibuf(accessor,
clip_index,
帧);
如果(orig_ibuf == NULL){
返回NULL;
}
....
如果(下标> 0){
如果(final_ibuf == orig_ibuf){
final_ibuf = IMB_dupImBuf(orig_ibuf);
}
IMB_scaleImBuf(final_ibuf,
ibuf-> x /(1 <<降级),// <=
ibuf-> y /(1 <<降级)); // <=
}
....
如果(input_mode == LIBMV_IMAGE_MODE_RGBA){
BLI_assert(ibuf-> channels == 3 || // <=
ibuf-> channels == 4); // <=
}
....
返回final_ibuf;
}

警告:

  • V522可能会取消引用空指针“ ibuf”。 tracking_util.c 765
  • V522可能会取消引用空指针“ ibuf”。 tracking_util.c 766
  • V522可能会取消引用空指针“ ibuf”。 tracking_util.c 783

在上面给出的片段中,您可以看到对ibuf变量的检查比在创建对象时使用此变量要早得多。 我们可能会在这里停止并确认指针取消引用的事实。 同时,如果我们对代码及其注释进行更艰苦的检查,我们将发现出现此错误的真正原因。 再次,这是一个错字。 在分析器指示的片段中,程序员应该使用变量orig_ibuf代替ibuf

变量类型错误

  typedef枚举eOutlinerIdOpTypes { 
OUTLINER_IDOP_INVALID = 0,
OUTLINER_IDOP_UNLINK,
OUTLINER_IDOP_LOCAL,
....
} eOutlinerIdOpTypes;
  typedef枚举eOutlinerLibOpTypes { 
OL_LIB_INVALID = 0,
OL_LIB_RENAME,
OL_LIB_DELETE,
} eOutlinerLibOpTypes;
 静态int outliner_lib_operation_exec(....) 
{
....
eOutlinerIdOpTypes事件; // <=
....
事件= RNA_enum_get(op-> ptr,“ type”);
切换(事件){
案例OL_LIB_RENAME:// <=
{
....
}
情况OL_LIB_DELETE:// <=
{
....
}
默认:
/ *无效-未处理* /
打破;
}
....
}

警告:

  • V556比较了不同枚举类型的值:switch(ENUM_TYPE_A){case ENUM_TYPE_B:…}。 outliner_tools.c 1286
  • V556比较了不同枚举类型的值:switch(ENUM_TYPE_A){case ENUM_TYPE_B:…}。 outliner_tools.c 1295

在此示例中,您可以看到两种类型的枚举。 可以预料的是,名称中几乎有相同的拼写错误。

实际上,该代码可以正常工作。 同时,它因类型不匹配而使我们感到困惑。 该变量获得一个枚举值,并与另一个变量的常量进行比较。 要更正此错误,只需将变量事件的类型更改为eOutlinerLibOpTypes即可

操作优先级错误

 静态无效blf_font_draw_buffer_ex(....) 
{
....
cbuf [3] =(unsigned char)((alphatest =((int)cbuf [3] +
(int)(a * 255))<255)吗? alphatest:255);
....
}

V593考虑查看“ A = B <C”类型的表达式。 该表达式的计算公式如下:“ A =(B <C)”。 blf_font.c 414

使用复杂表达式时,不遵守操作优先级是最常见的错误之一。 在这种情况下,这只是一个错字,但它违反了三元运算符的逻辑。 由于括号放置不正确,因此存在操作优先级错误。 最重要的是, alphatest变量的值也被损坏。 代替由三元运算符评估的值,为alphatest变量分配一个在比较操作结果中获得的bool-type值。 只有在此之后,三元运算符才能使用alphatest变量的值,并且结果不会保存。 要解决此错误,我们应按以下方式更改表达式:

  cbuf [3] =(unsigned char)(alphatest =(((int)cbuf [3] + 
(int)(a * 255))<255)吗? alphatest:255);

无效的常数

 布尔BKE_ffmpeg_alpha_channel_is_supported(RenderData * rd) 
{
int编解码器= rd-> ffcodecdata.codec;
如果(编解码器== AV_CODEC_ID_QTRLE)
返回true;
如果(编解码器== AV_CODEC_ID_PNG)
返回true;
如果(编解码器== AV_CODEC_ID_PNG)
返回true;
....
}

V649有两个带有相同条件表达式的’if’语句。 第一个’if’语句包含函数return。 这意味着第二个“ if”语句是毫无意义的。 检查行:1672、1675。writeffmpeg.c 1675

我们看到在单行条件的帮助下对变量值进行了连续检查以匹配该标志。 由于输入错误,其中一个标志被检查了两次。 很可能应该检查另一个常数,而不是重复检查。 这些常量有很多变体,这就是为什么很难说出如何修复此代码的原因。

在外部和内部循环中使用一个变量

  bool BM_face_exists_overlap_subset(....,const int len) 
{

....
对于(i = 0; i <len; i ++){
BM_ITER_ELEM(f,&viter,varr [i],BM_FACES_OF_VERT){
如果((f-> len <= len)&&(....)){
BMLoop * l_iter,* l_first;
 如果(is_init == false){ 
is_init = true;
对于(i = 0; i <len; i ++){// <=
BM_ELEM_API_FLAG_ENABLE(varr [i],_FLAG_OVERLAP);
}
}
....
}
}
}
}

V535变量“ i”用于该循环和外部循环。 检查行:2204,2212。bmesh_queries.c 2212

在外循环和内循环中使用相同的变量可能会导致外循环执行不正确。 在这种情况下,这不太可能是错误,因为循环可能正在寻找必要的元素并退出,并且仅在这种情况下才触发第二个循环。 但是,使用单个变量仍然是一个危险的技巧,如果有必要优化此代码片段,可能会导致实际错误。

冗余码

在任何程序中都可以找到过多的代码片段。 有时这是重构后被遗忘的旧代码。 但是有时,这些多余的片段会成为保持项目风格的一种方式。 这样的碎片可能非常危险。 换句话说,重复的代码通常表明存在逻辑错误。

再检查一遍

 静态无效刀_add_single_cut(....) 
{
....
如果((lh1-> v && lh2-> v)&& // <=
(lh1-> v-> v && lh2-> v && lh2-> v-> v)&& // <=
(e_base = BM_edge_exists(lh1-> v-> v,lh2-> v-> v))
{
....
返回;
}
....
}

V501’&&’运算符的左侧和右侧有相同的子表达式’lh2-> v’。 第781章

这是一个没有被深思熟虑的条件的变体之一。 当然这不是一个错误,只是一个额外的检查,但这并不意味着该代码不需要额外的检查。 条件由几个表达式组成。 同时,第二个表达式的一部分与第一个表达式中的一个变量的检查相同,因此此处不需要。 要修复此代码,我们需要从第二个表达式中删除过多的检查lh2-> v 。 之后,代码将变得更容易阅读。

另一个例子:

 静态整数edbm_rip_invoke__vert(....) 
{
....
如果(do_fill){
如果(do_fill){
....
}
}
....
}

V571定期检查。 “ if(do_fill)”条件已经在第751行中得到了验证。editmesh_rip.c 752

逻辑错误的另一个变体。 在外部和内部条件内检查绝对相同的表达式。 双重检查将始终给出相同的结果,这没有任何意义。 当然,此代码不会以任何方式影响程序的工作。 但是目前尚不清楚该代码将如何随着时间而变化,并且额外的检查会在将来误导一个人。

在项目的几个片段中可以找到不必要的检查。 这是分析仪检测到的另外几个点:

  • V571定期检查。 9587行已经验证了“ but”条件。interface_handlers.c 9590
  • V571定期检查。 ‘!me-> mloopcol’条件已在第252行中得到验证。paint_vertex.c 253
  • V571定期检查。 第5256行中的’constinv == 0’条件已经得到验证。transform_conversions.c 5257
  • V571定期检查。 ‘vlr-> v4’条件已在第4174行中得到验证。convertblender.c 4176
  • V571定期检查。 ‘ibuf ==((void *)0)’条件已在3557行中得到验证。sequencer.c 3559

第三个示例显然是冗余代码:

 静态void writedata_do_write(....) 
{
如果((wd == NULL)|| wd->错误||
(内存== NULL)|| memlen <1)返回;
如果(wd-> error)返回;
....
}

V649有两个带有相同条件表达式的’if’语句。 第一个’if’语句包含函数return。 这意味着第二个“ if”语句是毫无意义的。 检查行:331332。writefile.c 332

字符串if(wd-> error)返回; 过多,该函数将在处理此条件之前退出。 因此,应将其删除。

相反的条件块

 静态整数select_less_exec(....) 
{
....
如果((lastsel == 0)&&((bp-> hide == 0)&&((bp-> f1&SELECT)){
如果(lastsel!= 0)sel = 1;
否则sel = 0;
....
}
....
}

V637遇到两个相反的条件。 第二个条件始终为假。 检查行:938、939。editcurve_select.c 938

在片段中,我们可以看到外部条件块内还有一个额外的条件。 内部条件与主要条件相反,总是给出相同的结果。 sel变量永远不会为1。因此,简单地将sel = 0写入而无需进行附加检查就足够了。 尽管可以通过更改其中一个表达式来解决此错误。 由于我没有参与该项目的创建,因此很难确定。

多余的表达

  DerivedMesh * fluidsimModifier_do(....) 
{
....
如果(!fluidmd ||(fluidmd &&!fluidmd-> fss))
返回dm;
....
}

V728过度检查可以简化。 “ ||” 运算符被相反的表达式’!fluidmd’和’fluidmd’包围。 mod_fluidsim_util.c 528

在一个条件下检查一个和相同变量的相反值。 通常发现这些条件具有不同的种类和变化。 它们不会对软件造成任何损害,但是会使代码复杂化。 该表达式可以简化并编写如下:

 如果(!fluidmd ||!fluidmd-> fss))...。 

相似的片段:

  • V728过度检查可以简化。 “ ||” 运算符被相反的表达式’!render_only’和’render_only’包围。 drawobject.c 4663
  • V728过度检查可以简化。 “ ||” 运算符被相反的表达式“!parent”和“ parent”包围。 第1667章

另一个这样的条件:

 无效ED_transverts_create_from_obedit(....) 
{
....
if((tipsel && rootsel)||(rootsel)){....}
....
}

V686检测到一个模式:(rootsel)|| ((rootsel)&&…)。 表达式过多或包含逻辑错误。 第325章

如上例所示,同一变量在一个表达式中被检查两次。 此表达式不是错误的表达式,但是肯定有一个额外的检查。 让我们对其进行简化,使其更紧凑,更易于阅读。

 如果((tipsel || rootsel){....} 

在项目的其他地方有这样的错误。

  • V686检测到一个模式:(!py_b_len)|| ((!py_b_len)&&…)。 表达式过多或包含逻辑错误。 aud_pyapi.cpp 864
  • V686检测到一个模式:(xn == 0.0f)|| ((xn == 0.0f)&&…)。 表达式过多或包含逻辑错误。 renderdatabase.c 993
  • V686检测到一个模式:(xn == 0.0f)|| ((xn == 0.0f)&&…)。 表达式过多或包含逻辑错误。 renderdatabase.c 1115

迭代分配

 静态布尔find_prev_next_keyframes(....) 
{
....
做{
aknext =(ActKeyColumn *)BLI_dlrbTree_search_next(
&keys,compare_ak_cfraPtr,&cfranext);
如果(aknext){
如果(CFRA ==(int)aknext-> cfra){
cfranext = aknext-> cfra; // <-
}
其他{
如果(++ nextcount == U.view_frame_keyframes)
donenext = true;
}
cfranext = aknext-> cfra; // <-
}
} while(((aknext!= NULL)&&(donenext == false));
....
}

V519连续两次给’cfranext’变量赋值。 也许这是一个错误。 检查行:447、454。anim_draw.c 454

有条件的块中的分配没有意义,因为它的值在循环末尾没有任何条件的情况下再次分配。 在给定片段之后的代码中放置一个循环,有助于我们得出以下结论:多余的字符串位于上方。 它仅在prev变量和条件中不存在此字符串方面有所不同。 此外,假设多余的字符串在下面,并且条件CFRA ==(int)aknext-> cfra变为false,则此循环将变为无限循环。 这个片段确实需要一些修复,但是要如何精确地进行修复-只有项目的开发人员才知道。

多余或未使用的变量

项目中有许多这样的片段带有已初始化但未使用的变量。 其中一些可以看作是逻辑错误和过度检查,但我们已经对它们进行了很多讨论。 还有一些常数可能应该在函数内部进行更改。 但是结果是它们只是检查,总是返回相同的结果。 此类片段的示例:

 静态int rule_avoid_collision(....) 
{
....
int n,邻居= 0,最近= 0; // <=
....
如果(ptn &&最近== 0)// <=
MEM_freeN(ptn);

返回ret
}

V560条件表达式的一部分始终为true:最接近==0。boids.c 361

我将其他片段作为列表提供。 也许其中一些值得商bat,但它们值得关注。

  • V560条件表达式的一部分始终为true:edit ==0。particle.c 3781
  • V560条件表达式的一部分始终为true:!error。 点缓存.c 154
  • V560条件表达式的一部分始终为true:!error。 点缓存.c 2742
  • V560条件表达式的一部分始终为false:col。 drawobject.c 7803
  • V560条件表达式的一部分始终为false:!canvas_verts。 dynamicpaint.c 4636
  • V560条件表达式的一部分始终为true:(!leaf)。 octree.cpp 2513
  • V560条件表达式的一部分始终为true:(!leaf)。 octree.cpp 2710
  • V560条件表达式的一部分始终为false:(1 == i)。 basicstrokeshaders.cpp 67
  • V560条件表达式的一部分始终为true:(0 == i)。 basicstrokeshaders.cpp 69
  • V560条件表达式的一部分始终为false:(1 == i)。 basicstrokeshaders.cpp 84
  • V560条件表达式的一部分始终为true:(0 == i)。 basicstrokeshaders.cpp 86
  • V560条件表达式的一部分始终为false:(1 == i)。 basicstrokeshaders.cpp 155
  • V560条件表达式的一部分始终为true:(0 == i)。 第157章
  • V560条件表达式的一部分始终为true:(!radmod)。 第557章
  • V560条件表达式的一部分始终为true:完成!= 1. context.c 301
  • V560条件表达式的一部分始终为true:is_tablet == false。 ghost_systemwin32.cpp 665
  • V560条件表达式的一部分始终为true:mesh> =0。kx_gameobject.cpp 976

额外清除清单

  int TileManager :: gen_tiles(布尔切片) 
{
....
state.tiles.clear(); // <=
....
int tile_index = 0;
  state.tiles.clear(); 
state.tiles.resize(num);
....
}

V586两次调用“清除”功能以释放同一资源。 检查线:149、156。tile.cpp 156

在这种情况下,可能只是多余的一行。 在两个列表清除之间可能曾经有一些代码,但是在这种情况下,这只是另一个无用的片段,应删除该片段,以使代码不会混乱。 该字符串可能是由于应清除其中的其他对象这一事实的结果,乍一看不到。 在这种情况下,片段将是一个实际错误,可能会导致程序出现意外结果。

这种看似多余的代码通常会导致非常严重的错误,或者我的帮助在以后进行进一步修改时避免了它们。 这就是为什么您应该注意这些分析仪警告,而不要将其标记为“不重要”的原因。

阴谋

PVS-Studio团队现在正在积极致力于发展的新方向。 我在后面打掩护,用一些有关重新检查一些开源项目的文章来填充信息领域。 我们正在谈论的方向是什么? 我不能说。 我在这里只留下一张图片,您可以随意解释。

结论

分析仪在项目中检测到很多麻烦的地方。 但是,有时候,Blender中的编码风格很奇怪,我们不能肯定地说这些是错误的。 我认为,由于错别字经常会发生危险的错误。 PVS-Studio特别擅长捕获此类错误。 本文中描述的错误反映了作者的主观个人观点。 要查看分析仪的所有功能,请下载并自己尝试。

亚历山大·基比索夫(Alexander Chibisov)