Geekly articles weekly

LibreOffice：会计的噩梦


LibreOffice是功能强大的办公套件，可免费用于私人，教育和商业用途。 它的开发人员正在制作出色的产品，该产品在许多领域都可以用作Microsoft Office的替代产品。 PVS-Studio团队始终对查看此类知名项目的代码并试图发现其中的错误感兴趣。 这次很容易做到。 该项目包含许多可能导致严重问题的错误。 本文将讨论代码中发现的一些有趣的缺陷。

引言


LibreOffice是一个非常大的C ++项目。 对于开发团队来说，维持如此规模的项目是一项艰巨的任务。 而且，不幸的是，给人的印象是LibreOffice代码质量未能引起足够的重视。

一方面，该项目非常庞大，并非每个静态或动态分析工具都能处理13k源代码文件的分析。 与第三方库一起构建Office套件涉及许多文件。 LibreOffice主存储库存储约8k源代码文件。 如此大量的代码不仅会给开发人员带来问题：


另一方面，该项目有许多用户，需要找到并修复尽可能多的错误。 每个错误都会伤害成千上万的用户。 因此，庞大的代码库不应成为拒绝使用某些可以检测错误的工具的借口。 我认为读者已经猜到了我们在谈论静态代码分析器:)。

是的，使用静态分析器不能保证项目中没有错误。 但是，诸如PVS-Studio之类的工具能够在开发阶段发现大量错误，从而减少了与调试和项目支持相关的工作量。

让我们看看使用PVS-Studio静态代码分析器在LibreOffice源代码中有什么有趣的地方。 运行分析仪的可能性非常广泛：Windows，Linux，macOS。 为了撰写此评论，使用了在Windows上分析项目期间创建的PVS-Studio报告。

自2015年上次检查以来的变化




2015年3月，使用PVS-Studio 对LibreOffice进行了首次分析（“ LibreOffice项目的验证 ”）。 从那时起，办公套件作为一种产品有了很大的发展，但其中还包含许多错误。 从那以后，某些错误模式根本没有改变。 例如，这是第一篇文章中的一个错误：

V656变量“ aVRP”，“ aVPN”通过对同一函数的调用进行初始化。 可能是错误或未优化的代码。 考虑检查“ rSceneCamera.GetVRP（）”表达式。 检查行：177，178。viewcontactofe3dscene.cxx 178

void ViewContactOfE3dScene::createViewInformation3D(....) { .... const basegfx::B3DPoint aVRP(rSceneCamera.GetVRP()); const basegfx::B3DVector aVPN(rSceneCamera.GetVRP()); // <= const basegfx::B3DVector aVUV(rSceneCamera.GetVUV()); .... }

该错误已得到修复，但是在最新版本的代码中发现了以下错误：

V656变量“ aSdvURL”，“ aStrURL”通过对同一函数的调用来初始化。 可能是错误或未优化的代码。 考虑检查“ pThm-> GetSdvURL（）”表达式。 检查行：658，659。gallery1.cxx 659

 const INetURLObject& GetThmURL() const { return aThmURL; } const INetURLObject& GetSdgURL() const { return aSdgURL; } const INetURLObject& GetSdvURL() const { return aSdvURL; } const INetURLObject& GetStrURL() const { return aStrURL; } bool Gallery::RemoveTheme( const OUString& rThemeName ) { .... INetURLObject aThmURL( pThm->GetThmURL() ); INetURLObject aSdgURL( pThm->GetSdgURL() ); INetURLObject aSdvURL( pThm->GetSdvURL() ); INetURLObject aStrURL( pThm->GetSdvURL() ); // <= .... }

您可能已经注意到，微妙的复合函数名称仍然是错误的根源。

旧代码中的另一个有趣的示例：

V656变量“ nDragW”，“ nDragH”通过对同一函数的调用进行初始化。 可能是错误或未优化的代码。 考虑检查“ rMSettings.GetStartDragWidth（）”表达式。 检查行：471，472。winproc.cxx 472

 class VCL_DLLPUBLIC MouseSettings { .... long GetStartDragWidth() const; long GetStartDragHeight() const; .... } bool ImplHandleMouseEvent( .... ) { .... long nDragW = rMSettings.GetStartDragWidth(); long nDragH = rMSettings.GetStartDragWidth(); .... }

这段代码确实包含一个现已修复的错误。 但是代码中的错误并没有变得越来越小……现在已经发现了类似的情况：

V656变量“ defaultZoomX”，“ defaultZoomY”通过对同一函数的调用进行初始化。 可能是错误或未优化的代码。 考虑检查“ pViewData-> GetZoomX（）”表达式。 检查行：5673，5674。gridwin.cxx 5674

 OString ScGridWindow::getCellCursor(....) const { .... SCCOL nX = pViewData->GetCurX(); SCROW nY = pViewData->GetCurY(); Fraction defaultZoomX = pViewData->GetZoomX(); Fraction defaultZoomY = pViewData->GetZoomX(); // <= .... }

类比地从字面上将错误引入代码中。

不要上当




V765复合赋值表达式'x-= x-...'可疑。 考虑检查它是否存在错误。 swdtflvr.cxx 3509

 bool SwTransferable::PrivateDrop(...) { .... if ( rSrcSh.IsSelFrameMode() ) { //Hack: fool the special treatment aSttPt -= aSttPt - rSrcSh.GetObjRect().Pos(); } .... }

使用V765诊断程序发现了如此有趣的“ Hack”。 如果用注释简化一行代码，可能会得到意外的结果：

第一步：

 aSttPt = aSttPt - (aSttPt - rSrcSh.GetObjRect().Pos());

第二步：

 aSttPt = aSttPt - aSttPt + rSrcSh.GetObjRect().Pos();

第三步

 aSttPt = rSrcSh.GetObjRect().Pos();

那么什么是hack？

关于此主题的另一个示例：

V567相对于同一变量的另一个操作，'nCount'变量的修改没有顺序。 这可能导致不确定的行为。 stgio.cxx 214

 FatError EasyFat::Mark(....) { if( nCount > 0 ) { --nCount /= GetPageSize(); nCount++; } .... }

在这种情况下，代码执行可能取决于编译器和语言标准。 为什么不更轻松，更易理解，更可靠地重写此代码段？

如何不使用数组和向量




由于某种原因，有人在处理数组和向量时犯了很多类似的错误。 让我们看看这些例子。

V557阵列可能超限。 “ nPageNum”索引指向数组界限之外。 pptx-epptooxml.cxx 1168

 void PowerPointExport::ImplWriteNotes(sal_uInt32 nPageNum) { .... // add slide implicit relation to notes if (mpSlidesFSArray.size() >= nPageNum) addRelation(mpSlidesFSArray[ nPageNum ]->getOutputStream(), oox::getRelationship(Relationship::NOTESSLIDE), OUStringBuffer() .append("../notesSlides/notesSlide") .append(static_cast<sal_Int32>(nPageNum) + 1) .append(".xml") .makeStringAndClear()); .... }

最后一个有效索引应为等于size（）-1的值 。 但是在此代码段中，允许nPageNum索引的值可以为mpSlidesFSArray.size（）的情况 ，由于该原因，存在一个超出范围的数组，并且可以使用由“垃圾”组成的元素。

V557阵列可能超限。 'mnSelectedMenu'索引指向数组边界之外。 checklistmenu.cxx 826

 void ScMenuFloatingWindow::ensureSubMenuNotVisible() { if (mnSelectedMenu <= maMenuItems.size() && maMenuItems[mnSelectedMenu].mpSubMenuWin && maMenuItems[mnSelectedMenu].mpSubMenuWin->IsVisible()) { maMenuItems[mnSelectedMenu].mpSubMenuWin->ensureSubMenuNotVisible(); } EndPopupMode(); }

有趣的是，在此代码片段中，他们更清楚地编写了索引检查，但同时又犯了同样的错误。

V557阵列可能超限。 'nXFIndex'索引指向数组边界之外。 xestyle.cxx 2613

 sal_Int32 XclExpXFBuffer::GetXmlStyleIndex( sal_uInt32 nXFIndex ) const { OSL_ENSURE( nXFIndex < maStyleIndexes.size(), "...." ); if( nXFIndex > maStyleIndexes.size() ) return 0; // should be caught/debugged via above assert; return maStyleIndexes[ nXFIndex ]; }

这个错误是非常有趣的！ 在调试宏中，他们编写了正确的索引检查，而在另一个地方，他们又犯了一个错误，使他们可以离开数组。

现在考虑另一种与索引无关的错误。

V554不正确使用shared_ptr。 分配给“ new []”的内存将使用“ delete”清除。 dx_vcltools.cxx 158

 struct RawRGBABitmap { sal_Int32 mnWidth; sal_Int32 mnHeight; std::shared_ptr< sal_uInt8 > mpBitmapData; }; RawRGBABitmap bitmapFromVCLBitmapEx( const ::BitmapEx& rBmpEx ) { .... // convert transparent bitmap to 32bit RGBA // ======================================== const ::Size aBmpSize( rBmpEx.GetSizePixel() ); RawRGBABitmap aBmpData; aBmpData.mnWidth = aBmpSize.Width(); aBmpData.mnHeight = aBmpSize.Height(); aBmpData.mpBitmapData.reset( new sal_uInt8[ 4*aBmpData.mnWidth *aBmpData.mnHeight ] ); .... }

这段代码包含导致未定义程序行为的错误。 事实是，内存以不同的方式分配和释放。 为了适当地释放内存，您必须声明一个这样的类字段：

 std::shared_ptr< sal_uInt8[] > mpBitmapData;

如何两次制作宏




V568奇怪的是sizeof（）运算符的参数是'bTextFrame？ aProps：aShapeProps的表达式。 wpscontext.cxx 134

 oox::core::ContextHandlerRef WpsContext::onCreateContext(....) { .... OUString aProps[] = { .... }; OUString aShapeProps[] = { .... }; for (std::size_t i = 0; i < SAL_N_ELEMENTS(bTextFrame ? aProps : aShapeProps); //1 ++i) if (oInsets[i]) xPropertySet->setPropertyValue((bTextFrame ? aProps : aShapeProps)[i], //2 uno::makeAny(*oInsets[i])); .... }

不幸的是，对于许多开发人员而言，宏参数的行为不像函数参数。 忽略这一事实通常会导致错误。 在情况1和情况2中，使用三元运算符使用几乎相同的结构。 但是在第一种情况下-一个宏，在第二种情况下-一个函数。 但是，这只是问题的重中之重。

在情况1中，分析仪实际上检测到以下错误代码：

 for (std::size_t i = 0; i < (sizeof (bTextFrame ? aProps : aShapeProps) / sizeof ((bTextFrame ? aProps : aShapeProps)[0])); ++i)

这是我们使用SAL_N_ELEMENTS宏的循环。 sizeof运算符不评估三元运算符中的表达式。 在这种情况下，将使用指针的大小执行算术运算，其结果是与所指示数组的实际大小相距甚远的值。 错误值的计算还受到应用程序位深度的影响。

但是事实证明，这里有2个SAL_N_ELEMENTS宏！ 即 预处理器打开了错误的宏，这怎么可能？ 宏定义和开发人员评论将帮助我们：

 #ifndef SAL_N_ELEMENTS # if defined(__cplusplus) && ( defined(__GXX_EXPERIMENTAL_CXX0X__) || __cplusplus >= 201103L ) /* * Magic template to calculate at compile time the number of elements * in an array. Enforcing that the argument must be a array and not * a pointer, eg * char *pFoo="foo"; * SAL_N_ELEMENTS(pFoo); * fails while * SAL_N_ELEMENTS("foo"); * or * char aFoo[]="foo"; * SAL_N_ELEMENTS(aFoo); * pass * * Unfortunately if arr is an array of an anonymous class then we need * C++0x, ie see * http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#757 */ template <typename T, size_t S> char (&sal_n_array_size( T(&)[S] ))[S]; # define SAL_N_ELEMENTS(arr) (sizeof(sal_n_array_size(arr))) # else # define SAL_N_ELEMENTS(arr) (sizeof (arr) / sizeof ((arr)[0])) # endif #endif

宏的另一个版本包含一个安全的模板函数，但是出了点问题：

  1. 安全宏未包含在代码中；
  2. 仍然不可能使用另一个宏，因为 仅当将相同大小的数组转移到三元运算符时，才能成功实例化模板函数。 在这种情况下，使用此类宏将失去其意义。

错别字和复制粘贴




V1013类似比较序列中的可疑子表达式f1.Pitch == f2.CharSet。 xmldlg_export.cxx 1251

 inline bool equalFont( Style const & style1, Style const & style2 ) { awt::FontDescriptor const & f1 = style1._descr; awt::FontDescriptor const & f2 = style2._descr; return ( f1.Name == f2.Name && f1.Height == f2.Height && f1.Width == f2.Width && f1.StyleName == f2.StyleName && f1.Family == f2.Family && f1.CharSet == f2.CharSet && // <= f1.Pitch == f2.CharSet && // <= f1.CharacterWidth == f2.CharacterWidth && f1.Weight == f2.Weight && f1.Slant == f2.Slant && f1.Underline == f2.Underline && f1.Strikeout == f2.Strikeout && f1.Orientation == f2.Orientation && bool(f1.Kerning) == bool(f2.Kerning) && bool(f1.WordLineMode) == bool(f2.WordLineMode) && f1.Type == f2.Type && style1._fontRelief == style2._fontRelief && style1._fontEmphasisMark == style2._fontEmphasisMark ); }

如果我们决定更新或扩展该错误，那么该错误是更新文章“ 邪恶生活在比较函数中 ”的值得考虑的选择。 我认为仅发现此类错误（通过f2.Pitch ）的可能性非常小。 你觉得呢

V501在'&&'运算符的左侧和右侧有相同的子表达式'mpTable [ocArrayColSep]！= MpTable [eOp]'。 Formulacompiler.cxx 632

 void FormulaCompiler::OpCodeMap::putOpCode(....) { .... case ocSep: bPutOp = true; bRemoveFromMap = (mpTable[eOp] != ";" && mpTable[ocArrayColSep] != mpTable[eOp] && mpTable[ocArrayColSep] != mpTable[eOp]); break; .... }

漫不经心地复制的结果就是这样一段代码。 也许再次简单地重复了条件表达式，但是在代码中仍然没有这样的歧义。

V517检测到使用'if（A）{...} else if（A）{...}'模式。 存在逻辑错误的可能性。 检查行：781、783。mysqlc_databasemetadata.cxx 781

 Reference<XResultSet> SAL_CALL ODatabaseMetaData::getColumns(....) { .... bool bIsCharMax = !xRow->wasNull(); if (sDataType.equalsIgnoreAsciiCase("year")) nColumnSize = sColumnType.copy(6, 1).toInt32(); else if (sDataType.equalsIgnoreAsciiCase("date")) // <= nColumnSize = 10; else if (sDataType.equalsIgnoreAsciiCase("date")) // <= nColumnSize = 8; else if (sDataType.equalsIgnoreAsciiCase("datetime") || sDataType.equalsIgnoreAsciiCase("timestamp")) nColumnSize = 19; else if (!bIsCharMax) nColumnSize = xRow->getShort(7); else nColumnSize = nCharMaxLen; .... }

复制条件表达式的结果是，代码中出现错误，因为从未设置变量nColumnSize的值8 。

V523'then '语句等效于'else'语句。 svdpdf.hxx 146

 /// Transform the rectangle (left, right, top, bottom) by this Matrix. template <typename T> void Transform(....) { .... if (top > bottom) top = std::max(leftTopY, rightTopY); else top = std::min(leftTopY, rightTopY); if (top > bottom) bottom = std::max(leftBottomY, rightBottomY); // <= else bottom = std::max(leftBottomY, rightBottomY); // <= }

这里的函数min（）和max（）混淆了。 当然，由于界面中的这种错字，某些东西的缩放比例很奇怪。

怪周期




V533可能在“ for”运算符内递增了错误的变量。 考虑查看“ i”。 javatypemaker.cxx 602

 void printConstructors(....) { .... for (std::vector< unoidl::SingleInterfaceBasedServiceEntity::Constructor:: Parameter >::const_iterator j(i->parameters.begin()); j != i->parameters.end(); ++i) { o << ", "; printType(o, options, manager, j->type, false); if (j->rest) { o << "..."; } o << ' ' << codemaker::java::translateUnoToJavaIdentifier( u2b(j->name), "param"); } .... }

循环中的表达式++ i看起来非常可疑。 也许那里应该有++ j 。

V756嵌套循环内未使用“ nIndex2”计数器。 考虑检查“ nIndex”计数器的用法。 treex.cxx 34

 SAL_IMPLEMENT_MAIN_WITH_ARGS(argc, argv) { OString sXHPRoot; for (int nIndex = 1; nIndex != argc; ++nIndex) { if (std::strcmp(argv[nIndex], "-r") == 0) { sXHPRoot = OString( argv[nIndex + 1] ); for( int nIndex2 = nIndex+3; nIndex2 < argc; nIndex2 = nIndex2 + 2 ) { argv[nIndex-3] = argv[nIndex-1]; argv[nIndex-2] = argv[nIndex]; } argc = argc - 2; break; } } common::HandledArgs aArgs; if( !common::handleArguments(argc, argv, aArgs) ) { WriteUsage(); return 1; } .... }

内部 for循环中存在某种错误。 因为 nIndex变量不会改变；数组的相同两个元素在每次迭代时都会被覆盖。 变量nIndex2最有可能代替nIndex 而已在所有地方使用。

V1008考虑检查“ for”运算符。 最多执行一次循环迭代。 diagramhelper.cxx 292

 void DiagramHelper::setStackMode( const Reference< XDiagram > & xDiagram, StackMode eStackMode ) { .... sal_Int32 nMax = aChartTypeList.getLength(); if( nMax >= 1 ) nMax = 1; for( sal_Int32 nT = 0; nT < nMax; ++nT ) { uno::Reference< XChartType > xChartType( aChartTypeList[nT] ); .... } .... }

for循环有意限制为1次迭代。 目前尚不清楚为什么要这样进行。

V612循环内无条件的“返回”。 pormulti.cxx 891

 SwTextAttr const* MergedAttrIterMulti::NextAttr(....) { .... SwpHints const*const pHints(m_pNode->GetpSwpHints()); if (pHints) { while (m_CurrentHint < pHints->Count()) { SwTextAttr const*const pHint(pHints->Get(m_CurrentHint)); ++m_CurrentHint; rpNode = m_pNode; return pHint; } } return nullptr; .... }

一个迭代产生的简单怪异循环的示例，最好将其重写为条件语句。

还有一些这样的地方：

  • V612循环内无条件的“返回”。 txtfrm.cxx 144
  • V612循环内无条件的“返回”。 txtfrm.cxx 202
  • V612循环内无条件的“返回”。 txtfrm.cxx 279

奇怪的条件




V637遇到两个相反的条件。 第二个条件始终为假。 检查行：281、285。authfld.cxx 281

 sal_uInt16 SwAuthorityFieldType::GetSequencePos(sal_IntPtr nHandle) { .... SwTOXSortTabBase* pOld = aSortArr[i].get(); if(*pOld == *pNew) { //only the first occurrence in the document //has to be in the array if(*pOld < *pNew) pNew.reset(); else // remove the old content aSortArr.erase(aSortArr.begin() + i); break; } .... }

分析器发现比较结果冲突。 这段代码显然有问题。

在此位置可以看到相同的代码：

  • V637遇到两个相反的条件。 第二个条件始终为假。 检查行：1827、1829。doctxm.cxx 1827

V590考虑检查此表达式。 表达式过多或打印错误。 fileurl.cxx 55

 OUString convertToFileUrl(char const * filename, ....) { .... if ((filename[0] == '.') || (filename[0] != SEPARATOR)) { .... } .... }

上面的代码片段的问题在于，第一个条件表达式不会影响整个表达式的结果。

基于这样的错误，我什至写了一篇理论文章：“ C / C ++中的逻辑表达式。专业人员怎么错 。”

V590考虑检查此表达式。 表达式过多或打印错误。 unoobj.cxx 1895

 uno::Sequence< beans::PropertyState > SwUnoCursorHelper::GetPropertyStates(....) { .... if (((SW_PROPERTY_STATE_CALLER_SWX_TEXT_PORTION == eCaller) || (SW_PROPERTY_STATE_CALLER_SWX_TEXT_PORTION_TOLERANT == eCaller)) && pEntry->nWID < FN_UNO_RANGE_BEGIN && pEntry->nWID > FN_UNO_RANGE_END && pEntry->nWID < RES_CHRATR_BEGIN && pEntry->nWID > RES_TXTATR_END ) { pStates[i] = beans::PropertyState_DEFAULT_VALUE; } .... }

我不立即了解这种情况的问题所在，因此，从预处理文件中写出了扩展的代码片段：

 if (((SW_PROPERTY_STATE_CALLER_SWX_TEXT_PORTION == eCaller) || (SW_PROPERTY_STATE_CALLER_SWX_TEXT_PORTION_TOLERANT == eCaller)) && pEntry->nWID < (20000 + 1600) && pEntry->nWID > ((20000 + 2400) + 199) && pEntry->nWID < 1 && pEntry->nWID > 63 ) { pStates[i] = beans::PropertyState_DEFAULT_VALUE; }

碰巧在条件中指定的4个范围中没有同时包含一个数字。 开发人员犯了一个错误。

V590考虑检查'* pData <= MAXLEVEL && * pData <= 9'表达式。 表达式过多或打印错误。 ww8par2.cxx 756

 const sal_uInt8 MAXLEVEL = 10; void SwWW8ImplReader::Read_ANLevelNo(....) { .... // Range WW:1..9 -> SW:0..8 no bullets / numbering if (*pData <= MAXLEVEL && *pData <= 9) { .... } else if( *pData == 10 || *pData == 11 ) { // remember type, the rest happens at Sprm 12 m_xStyles->mnWwNumLevel = *pData; } .... }

由于第一个条件使用值为10的常数，因此该条件被证明是多余的。 这段代码可以重写如下：

 if (*pData <= 9) { .... } else if( *pData == 10 || *pData == 11 ) { .... }

但是也许所提供的代码中还有另一个问题。

V757使用'dynamic_cast'进行类型转换后，可能会将不正确的变量与nullptr进行比较。 检查行：2709、2710。menu.cxx 2709

 void PopupMenu::ClosePopup(Menu* pMenu) { MenuFloatingWindow* p = dynamic_cast<MenuFloatingWindow*>(ImplGetWindow()); PopupMenu *pPopup = dynamic_cast<PopupMenu*>(pMenu); if (p && pMenu) p->KillActivePopup(pPopup); }

条件很可能包含错误。 有必要检查指针p和pPopup 。

V668没有针对空测试'm_pStream'指针的意义，因为使用'new'运算符分配了内存。 如果内存分配错误，将生成异常。 zipfile.cxx 408

 ZipFile::ZipFile(const std::wstring &FileName) : m_pStream(nullptr), m_bShouldFree(true) { m_pStream = new FileStream(FileName.c_str()); if (m_pStream && !isZipStream(m_pStream)) { delete m_pStream; m_pStream = nullptr; } }

分析器检测到新操作符返回的指针的值与零进行比较的情况。 根据C ++语言标准，如果无法进行内存分配，则新运算符将引发异常std :: bad_alloc 。 LibreOffice项目中仅发现了45个这样的地方，对于如此大量的代码而言，这是很小的。 但这仍然会给用户带来麻烦。 开发人员应删除不必要的检查或通过以下方式创建对象：

 m_pStream = new (std::nothrow) FileStream(FileName.c_str());

V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 工具箱2.cxx 1042

 void ToolBox::SetItemImageMirrorMode( sal_uInt16 nItemId, bool bMirror ) { ImplToolItems::size_type nPos = GetItemPos( nItemId ); if ( nPos != ITEM_NOTFOUND ) { ImplToolItem* pItem = &mpData->m_aItems[nPos]; if ((pItem->mbMirrorMode && !bMirror) || // <= (!pItem->mbMirrorMode && bMirror)) // <= { .... } } }

很久以前， V728诊断程序已扩展到很可能不是错误的情况，而是使代码复杂化的情况。 而且在复杂的代码中，迟早仍会出错。

此条件简化为以下内容：

 if (pItem->mbMirrorMode != bMirror) { .... }

在该项目中发现了大约60个类似的结构，其中一些非常非常庞大。 该项目的作者应熟悉PVS-Studio分析仪的完整报告。

安全问题




V523'then '语句等效于'else'语句。 docxattributeoutput.cxx 1571

 void DocxAttributeOutput::DoWritePermissionTagEnd( const OUString & permission) { OUString permissionIdAndName; if (permission.startsWith("permission-for-group:", &permissionIdAndName)) { const sal_Int32 sparatorIndex = permissionIdAndName.indexOf(':'); const OUString permissionId = permissionIdAndName.copy(....); const OString rId = OUStringToOString(....).getStr(); m_pSerializer->singleElementNS(XML_w, XML_permEnd, FSNS(XML_w, XML_id), rId.getStr(), FSEND); } else { const sal_Int32 sparatorIndex = permissionIdAndName.indexOf(':'); const OUString permissionId = permissionIdAndName.copy(....); const OString rId = OUStringToOString(....).getStr(); m_pSerializer->singleElementNS(XML_w, XML_permEnd, FSNS(XML_w, XML_id), rId.getStr(), FSEND); } }

这里复制了一大段代码。 对于更改某些权限的功能，发现的问题看起来非常可疑。

V1001分配了“ DL”变量，但在功能结束时未使用。 cipher.cxx 811

 static void BF_updateECB( CipherContextBF *ctx, rtlCipherDirection direction, const sal_uInt8 *pData, sal_uInt8 *pBuffer, sal_Size nLength) { CipherKeyBF *key; sal_uInt32 DL, DR; key = &(ctx->m_key); if (direction == rtl_Cipher_DirectionEncode) { RTL_CIPHER_NTOHL64(pData, DL, DR, nLength); BF_encode(key, &DL, &DR); RTL_CIPHER_HTONL(DL, pBuffer); RTL_CIPHER_HTONL(DR, pBuffer); } else { RTL_CIPHER_NTOHL(pData, DL); RTL_CIPHER_NTOHL(pData, DR); BF_decode(key, &DL, &DR); RTL_CIPHER_HTONL64(DL, DR, pBuffer, nLength); } DL = DR = 0; }

DL和DR变量通过在函数末尾的简单赋值而无效，并且不再使用。 对于编译器，这是删除命令以进行优化的借口。

要正确清除Windows应用程序中的变量，可以使用以下方式重写代码：

 RtlSecureZeroMemory(&DL, sizeof(DL)); RtlSecureZeroMemory(&DR, sizeof(DR));

但是对于LibreOffice，这里需要一个跨平台的解决方案。

来自另一个功能的类似警告：
  • V1001分配了“ DL”变量，但在功能结束时未使用。 cipher.cxx 860

V764传递给'queryStream'函数的参数的可能错误顺序：'rUri'和'rPassword'。 tdoc_storage.cxx 271

 css::uno::Reference< css::io::XStream > queryStream( const css::uno::Reference< css::embed::XStorage > & xParentStorage, const OUString & rPassword, const OUString & rUri, StorageAccessMode eMode, bool bTruncate ); uno::Reference< io::XOutputStream > StorageElementFactory::createOutputStream( const OUString & rUri, const OUString & rPassword, bool bTruncate ) { .... uno::Reference< io::XStream > xStream = queryStream( xParentStorage, rUri, rPassword, READ_WRITE_CREATE, bTruncate ); .... }

请注意，在queryStream函数中，在参数列表中，首先是rPassword ，然后是rUri 。 在代码中有一个地方，当调用此函数时，相应的参数混合在一起。

V794应该保护赋值运算符免受'this ==＆rToBeCopied'的影响。 hommatrixtemplate.hxx 121

 ImplHomMatrixTemplate& operator=(....) { // complete initialization using copy for(sal_uInt16 a(0); a < (RowSize - 1); a++) { memcpy(&maLine[a], &rToBeCopied.maLine[a], sizeof(....)); } if(rToBeCopied.mpLine) { mpLine.reset( new ImplMatLine< RowSize >(....) ); } return *this; }

这种情况更多是关于安全代码编写。 复制语句中没有检查要为其分配对象的检查。 该项目总共有大约30种这样的复制运算符实现。

SysAllocString错误




V649有两个带有相同条件表达式的'if'语句。 第一个'if'语句包含函数return。 这意味着第二个“ if”语句是毫无意义的。 检查行：125、137。acctable.cxx 137

 STDMETHODIMP CAccTable::get_columnDescription(long column, BSTR * description) { SolarMutexGuard g; ENTER_PROTECTED_BLOCK // #CHECK# if(description == nullptr) return E_INVALIDARG; // #CHECK XInterface# if(!pRXTable.is()) return E_FAIL; .... SAFE_SYSFREESTRING(*description); *description = SysAllocString(o3tl::toW(ouStr.getStr())); if(description==nullptr) // <= return E_FAIL; return S_OK; LEAVE_PROTECTED_BLOCK }

SysAllocString（）函数返回的指针可以为NULL 。 这段代码的作者写了一些奇怪的东西。 未检查指向已分配内存的指针，这可能导致程序出现问题。

其他功能的类似警告：

  • V649有两个带有相同条件表达式的'if'语句。 第一个'if'语句包含函数return。 这意味着第二个“ if”语句是毫无意义的。 检查行：344、356。acctable.cxx 356
  • V649有两个带有相同条件表达式的'if'语句。 第一个'if'语句包含函数return。 这意味着第二个“ if”语句是毫无意义的。 检查行：213、219。trvlfrm.cxx 219

V530需要使用功能'SysAllocString'的返回值。 maccessible.cxx 1077

 STDMETHODIMP CMAccessible::put_accValue(....) { .... if(varChild.lVal==CHILDID_SELF) { SysAllocString(m_pszValue); m_pszValue=SysAllocString(szValue); return S_OK; } .... }

不使用对SysAllocString（）函数的调用之一的结果。 开发人员应注意这段代码。

杂项


V716 return语句中的可疑类型转换：返回了HRESULT，但函数实际上返回了BOOL。 maccessible.cxx 2649

 BOOL CMAccessible::get_IAccessibleFromXAccessible(....) { ENTER_PROTECTED_BLOCK // #CHECK# if(ppIA == nullptr) { return E_INVALIDARG; // <= } BOOL isGet = FALSE; if(g_pAgent) isGet = g_pAgent->GetIAccessibleFromXAccessible(....); if(isGet) return TRUE; else return FALSE; LEAVE_PROTECTED_BLOCK }

函数执行的一个分支返回一个值，该值的类型与函数的返回值的类型不匹配。HRESULT类型具有比BOOL类型更复杂的格式，并用于存储操作状态。例如，E_INVALIDARG的值为0x80070057L。像这样写是正确的：

 return FAILED(E_INVALIDARG);

您可以在V716诊断文档中阅读有关此内容的更多信息。

还有更多类似的地方：

  • V716 return语句中的可疑类型转换：返回了HRESULT，但函数实际上返回了BOOL。第1299章
  • V716 return语句中的可疑类型转换：返回了HRESULT，但函数实际上返回了BOOL。maccessible.cxx 2660

V670未初始化的类成员'm_aMutex'用于初始化'm_aModifyListeners'成员。请记住，成员是按照类内声明的顺序进行初始化的。fmgridif.cxx 1033

 FmXGridPeer::FmXGridPeer(const Reference< XComponentContext >& _rxContext) :m_aModifyListeners(m_aMutex) ,m_aUpdateListeners(m_aMutex) ,m_aContainerListeners(m_aMutex) ,m_aSelectionListeners(m_aMutex) ,m_aGridControlListeners(m_aMutex) ,m_aMode( getDataModeIdentifier() ) ,m_nCursorListening(0) ,m_bInterceptingDispatch(false) ,m_xContext(_rxContext) { // Create must be called after this constructor m_pGridListener.reset( new GridListenerDelegator( this ) ); } class __declspec(dllexport) FmXGridPeer: public cppu::ImplInheritanceHelper<....> { .... ::comphelper::OInterfaceContainerHelper2 m_aModifyListeners, m_aUpdateListeners, m_aContainerListeners, m_aSelectionListeners, m_aGridControlListeners; .... protected: css::uno::Reference< css::uno::XComponentContext > m_xContext; ::osl::Mutex m_aMutex; .... };

根据语言标准，构造函数中类成员的初始化顺序按照其在类中的声明顺序进行。在我们的例子中，m_aMutex字段将在参与该类的其他五个字段的初始化之后进行初始化。

以下是其中一个类字段的构造函数：

 OInterfaceContainerHelper2( ::osl::Mutex & rMutex );

互斥对象通过引用传递。在这种情况下，可能会出现各种问题：从访问未初始化的内存到随后丢失对象更改。

V763参数“ nNativeNumberMode”始终在使用前在函数体中重写。calendar_jewish.cxx 286

 OUString SAL_CALL Calendar_jewish::getDisplayString(...., sal_Int16 nNativeNumberMode ) { // make Hebrew number for Jewish calendar nNativeNumberMode = NativeNumberMode::NATNUM2; if (nCalendarDisplayCode == CalendarDisplayCode::SHORT_YEAR) { sal_Int32 value = getValue(CalendarFieldIndex::YEAR) % 1000; return mxNatNum->getNativeNumberString(...., nNativeNumberMode ); } else return Calendar_gregorian::getDisplayString(...., nNativeNumberMode ); }

函数参数由于各种原因而被覆盖：对抗编译器警告，向后兼容性，拐杖等。但是，这些解决方案中的任何一个都不是很好，并且代码看起来很混乱。

您应该查看该地点的代码以及其他一些类似的代码：

  • V763参数“ bExtendedInfo”在使用前总是在功能体内重写。graphicfilter2.cxx 442
  • V763参数“ nVerbID”在使用前总是在函数体中重写。oleembed.cxx 841
  • V763 Parameter 'pCursor' is always rewritten in function body before being used. edlingu.cxx 843
  • V763 Parameter 'pOutput' is always rewritten in function body before being used. vnew.cxx 181
  • V763 Parameter 'pOutput' is always rewritten in function body before being used. vnew.cxx 256

结论


我个人使用产品后，出现了检查LibreOffice代码的愿望。由于某种原因，通过随机启动，该文本将从绝对所有菜单项中消失。仅保留图标和单调的条纹。该错误很可能是高级错误，并且可能无法借助静态分析工具找到该错误。尽管如此，分析器发现了许多与此无关的问题，如果LibreOffice开发人员关注静态代码分析器并尝试使用它们来提高项目的质量和可靠性，我将感到高兴。这将对每个人都有用。

谢谢您的关注。 订阅我们的频道，并随时关注来自编程领域的新闻！




如果您想与讲英语的读者分享这篇文章，请使用以下链接：Svyatoslav Razmyslov。LibreOffice：会计师的噩梦

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

More articles:


All Articles