LibreOffice: كابوس المحاسب

LibreOffice عبارة عن مجموعة مكتبية قوية مجانية للاستخدام الخاص والتعليمي والتجاري. يقوم مطوروها بعمل منتج رائع ، والذي يستخدم في العديد من المجالات كبديل لمايكروسوفت أوفيس. يهتم فريق PVS-Studio دائمًا بالنظر في رمز هذه المشاريع المعروفة ومحاولة العثور على أخطاء فيها. هذه المرة كان من السهل القيام به. يحتوي المشروع على العديد من الأخطاء التي يمكن أن تؤدي إلى مشاكل خطيرة. ستناقش المقالة بعض العيوب المثيرة للاهتمام الموجودة في التعليمات البرمجية.

مقدمة

LibreOffice هو مشروع C ++ كبير جدًا. يعد الحفاظ على مشروع بهذا الحجم مهمة صعبة لفريق التطوير. ولسوء الحظ ، يحصل المرء على انطباع بأن جودة كود LibreOffice فشلت في إيلاء اهتمام كاف.

من ناحية ، فإن المشروع ضخم للغاية ، ولا يمكن لكل أداة تحليل ثابتة أو ديناميكية التعامل مع تحليل ملفات التعليمات البرمجية المصدر 13k. تشارك العديد من الملفات في بناء مجموعة المكتب مع مكتبات الطرف الثالث. يخزن مستودع LibreOffice الرئيسي حوالي 8k من ملفات التعليمات البرمجية المصدر. هذا الكم من التعليمات البرمجية يخلق مشاكل ليس فقط للمطورين:

من ناحية أخرى ، يحتوي المشروع على العديد من المستخدمين ويحتاج إلى العثور على أكبر عدد ممكن من الأخطاء وإصلاحها. يمكن أن يضر كل خطأ مئات وآلاف المستخدمين. لذلك ، لا ينبغي أن يصبح الحجم الكبير لقاعدة التعليمات البرمجية عذرًا لرفض استخدام أدوات معينة يمكنها اكتشاف الأخطاء. أعتقد أن القارئ قد خمن بالفعل أننا نتحدث عن محللات الكود الثابتة :).

نعم ، لا يضمن استخدام أجهزة التحليل الثابتة عدم وجود أخطاء في المشروع. ومع ذلك ، فإن أدوات مثل PVS-Studio قادرة على العثور على عدد كبير من الأخطاء في مرحلة التطوير وبالتالي تقليل حجم العمل المرتبط بتصحيح الأخطاء ودعم المشروع.

دعنا نرى ما يمكن أن تجده مثيرًا للاهتمام في كود مصدر LibreOffice إذا كنت تأخذ محلل الكود الثابت PVS-Studio . إمكانيات تشغيل المحلل واسعة النطاق: Windows و Linux و macOS. لكتابة هذه المراجعة ، تم استخدام تقرير PVS-Studio ، تم إنشاؤه أثناء تحليل المشروع على Windows.

التغييرات منذ آخر فحص في عام 2015

في مارس 2015 ، تم إجراء التحليل الأول لـ LibreOffice (" التحقق من مشروع LibreOffice ") باستخدام PVS-Studio. منذ ذلك الحين ، تطورت مجموعة المكتب بشكل كبير كمنتج ، ولكن داخلها يحتوي أيضًا على العديد من الأخطاء. ولم تتغير بعض أنماط الخطأ على الإطلاق منذ ذلك الحين. هنا ، على سبيل المثال ، خطأ من المقالة الأولى:

تتم تهيئة المتغيرات 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 - ...' مشبوه. ضع في اعتبارك فحصه بحثًا عن خطأ محتمل. 3509 محمد علي الرواشدة

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

هنا تم العثور على "هاك" مثيرة للاهتمام باستخدام التشخيص V765 . إذا قمت بتبسيط سطر من التعليمات البرمجية مع تعليق ، يمكنك الحصول على نتيجة غير متوقعة:

الخطوة الأولى:

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

الخطوة الثانية:

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

الخطوة الثالثة

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

ثم ما هو الاختراق؟

مثال آخر على هذا الموضوع:

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

يجب أن يكون آخر فهرس صالح قيمة مساوية للحجم () - 1 . ولكن في جزء التعليمات البرمجية هذا ، تم السماح بموقف عندما يمكن أن يكون لفهرس nPageNum القيمة mpSlidesFSArray.size () ، بسبب وجود صفيف خارج الحدود والعمل مع عنصر يتكون من "القمامة".

V557 تجاوز الصفيف ممكن. يشير مؤشر "mnSelectedMenu" إلى تجاوز الصفيف. 826

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

ومن المثير للاهتمام أنه في جزء التعليمات البرمجية هذا كتبوا فحصًا للمؤشر بشكل أكثر وضوحًا ، ولكن في نفس الوقت ارتكبوا نفس الخطأ.

V557 تجاوز الصفيف ممكن. يشير مؤشر "nXFIndex" إلى تجاوز الصفيف. 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. 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 بتقييم التعبير في العامل الثلاثي. في هذه الحالة ، يتم الحساب باستخدام حجم المؤشرات ، والنتيجة هي قيم بعيدة عن الحجم الحقيقي للمصفوفات المحددة. بالإضافة إلى ذلك ، يتأثر حساب القيم غير الصحيحة بعمق البتات الخاص بالتطبيق.

ولكن اتضح بعد ذلك أن هناك وحدتان من وحدات الماكرو 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]' إلى اليسار وإلى يمين عامل التشغيل "&&". 632

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

كانت نتيجة النسخ غير المدروس قطعة من التعليمات البرمجية. ربما يتم تكرار التعبير الشرطي ببساطة مرة أخرى ، ولكن لا يزال في القانون لا يوجد مكان لمثل هذا الغموض.

V517 استخدام نمط 'if (A) {...} آخر إذا تم الكشف عن (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; .... } 

نتيجة لنسخ التعبيرات الشرطية ، حدث خطأ في التعليمات البرمجية ، ونتيجة لذلك لم يتم تعيين القيمة 8 للمتغير nColumnSize .

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

هناك نوع من الخطأ في الحلقة الداخلية . لأن لا يتغير متغير nIndex ؛ يتم استبدال نفس عنصري الصفيف عند كل تكرار. على الأرجح ، بدلاً من nIndex ، كان يجب استخدام المتغير nIndex2 في كل مكان .

V1008 خذ بعين الاعتبار فحص عامل التشغيل "for". لن يتم تنفيذ أكثر من تكرار للحلقة. مخطط 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 عمدا على تكرار واحد. ليس من الواضح لماذا يتم ذلك بهذه الطريقة.

V612 "عودة" غير مشروطة داخل حلقة. 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 "عودة" غير مشروطة داخل حلقة. 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 خذ بعين الاعتبار فحص هذا التعبير. التعبير زائد أو يحتوي على خطأ مطبعي. 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; } 

حدث ذلك أنه لم يتم تضمين رقم واحد في وقت واحد في النطاقات الأربعة المحددة في الشرط بالأرقام. ارتكب المطورون خطأ.

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 من الممكن مقارنة متغير غير صحيح مع nullptr بعد تحويل النوع باستخدام "dynamic_cast". خطوط التحقق: 2709 ، 2710. القائمة. 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" مقابل قيمة خالية ، حيث تم تخصيص الذاكرة باستخدام عامل التشغيل "الجديد". سيتم إنشاء الاستثناء في حالة خطأ تخصيص الذاكرة. 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 . تم العثور على 45 مكانًا فقط في مشروع LibreOffice ، وهو صغير جدًا لمثل هذا الحجم من التعليمات البرمجية. ولكن مع ذلك ، يمكن أن يؤدي هذا إلى مشاكل للمستخدمين. يجب على المطورين إزالة عمليات التحقق غير الضرورية أو إنشاء كائنات بهذه الطريقة:

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

V728 يمكن تبسيط الشيك الزائد. '(A &&! B) || التعبير (! A && B) "يعادل التعبير" bool (A)! = Bool (B) ". 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". دوكسي 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" ولكن لا يتم استخدامه بواسطة نهاية الوظيفة. 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" ولكن لا يتم استخدامه بواسطة نهاية الوظيفة. 860

V764 ترتيب غير صحيح للوسيطات التي تم تمريرها إلى دالة "queryStream": "rUri" و "rPassword". 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'. 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' الأولى على إرجاع دالة. هذا يعني أن عبارة "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' الأولى على إرجاع دالة. هذا يعني أن عبارة "if" الثانية لا معنى لها. تحقق من الخطوط: 344 ، 356. acctable.cxx 356
• V649 يوجد عبارتان 'if' مع تعبيرات شرطية متطابقة. تحتوي العبارة 'if' الأولى على إرجاع دالة. هذا يعني أن عبارة "if" الثانية لا معنى لها. خطوط التحقق: 213 ، 219. trvlfrm.cxx 219

V530 مطلوب القيمة المرجعة للدالة 'SysAllocString' لاستخدامها. ماكسيبل 1077

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

لا يتم استخدام نتيجة أحد الاستدعاءات للدالة SysAllocString () . يجب على المطورين الانتباه إلى هذا الجزء من التعليمات البرمجية.

متفرقات

V716 تحويل نوع مريب في بيان الإرجاع: إرجاع HRESULT ، لكن الدالة ترجع فعليًا BOOL. علي بن محمد 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 تحويل نوع مريب في بيان الإرجاع: إرجاع HRESULT ، لكن الدالة ترجع فعليًا BOOL. 1299
• V716 تحويل نوع مريب في بيان الإرجاع: إرجاع HRESULT ، لكن الدالة ترجع فعليًا BOOL. 2660

V670 يتم استخدام عضو الفئة غير المحدد "m_aMutex" لتهيئة عضو "m_aModifyListeners". تذكر أنه تتم تهيئة الأعضاء بترتيب إعلاناتهم داخل الفصل الدراسي. اف ام جريدف 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' في نص الوظيفة قبل استخدامها. 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' في نص الوظيفة قبل استخدامها. 442
• يتم دائمًا إعادة كتابة معلمة V763 'nVerbID' في نص الوظيفة قبل استخدامها. علي محمد علي 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 بمحللات الشفرة الثابتة وحاولوا استخدامها لتحسين جودة وموثوقية المشروع. سيكون مفيدا للجميع.

شكرا لكم على اهتمامكم. اشترك في قنواتنا و تابع أخبار عالم البرمجة!

• Vk: pvsstudio_rus
• برقية: pvsstudio_rus
• انستقرام : @ pvsstudio_rus
• تويتر: @ pvsstudio_rus
• يوتيوب: PVSStudioTool

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فيرجى استخدام رابط الترجمة: Svyatoslav Razmyslov. ليبر أوفيس: كابوس المحاسب