تحليل كود ROOT ، إطار تحليل البيانات العلمية

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

مقدمة


الصورة 1

ROOT عبارة عن مجموعة أدوات برامج علمية معيارية. يوفر جميع الوظائف اللازمة للتعامل مع معالجة البيانات الضخمة والتحليل الإحصائي والتصور والتخزين. هو مكتوب بشكل رئيسي في C ++. ولد ROOT في سيرن ، في قلب البحث عن فيزياء الطاقة العالية. كل يوم ، يستخدم الآلاف من علماء الفيزياء تطبيقات ROOT لتحليل بياناتهم أو لإجراء عمليات المحاكاة.

PVS-Studio هي أداة لاكتشاف أخطاء البرامج ونقاط الضعف المحتملة في التعليمات البرمجية المصدر للبرامج المكتوبة بلغات C و C ++ و C # و Java. إنه يعمل على أنظمة تشغيل Windows و Linux و macOS 64 بت ، ويمكنه تحليل شفرة المصدر المكتوبة لأنظمة ARM 32 بت و 64 بت و المضمنة.

لاول مرة تشخيص جديد


V1046 الاستخدام غير الآمن لأنواع bool و "int" معًا في العملية '& ='. GSLMultiRootFinder.h 175

int AddFunction(const ROOT::Math::IMultiGenFunction & func) { ROOT::Math::IMultiGenFunction * f = func.Clone(); if (!f) return 0; fFunctions.push_back(f); return fFunctions.size(); } template<class FuncIterator> bool SetFunctionList( FuncIterator begin, FuncIterator end) { bool ret = true; for (FuncIterator itr = begin; itr != end; ++itr) { const ROOT::Math::IMultiGenFunction * f = *itr; ret &= AddFunction(*f); } return ret; } 

أولاً ، إليك خطأ رائع عثر عليه في الإصدار التجريبي من PVS-Studio ، والذي كنت أستخدمه لهذا الاستعراض.

التوقعات. الدالة SetFunctionList traverses قائمة التكرار. إذا كانت هناك أداة تكرار واحدة على الأقل غير صالحة ، فتُرجع الدالة " خطأ" أو " صحيح" .

حقيقة واقعة. يمكن أن ترجع الدالة SetFunctionList false حتى بالنسبة للتكرارات الصالحة. دعونا معرفة السبب. ترجع الدالة AddFunction عدد التكرارات الصالحة في قائمة fFunctions . بمعنى أن إضافة برامج التكرار غير الفارغة ستؤدي إلى زيادة حجم القائمة بشكل متزايد: 1 و 2 و 3 و 4 وما إلى ذلك. هذا هو المكان الذي يلعب فيه الخطأ:

 ret &= AddFunction(*f); 

نظرًا لأن الدالة تُرجع قيمة type int بدلاً من bool ، ستُرجع العملية '& =' خطأ للقيم الزوجية لأنه يتم دائمًا تعيين أقل عدد من الأرقام الزوجية على الصفر. هذا هو كيف يمكن لخطأ واحد دقيق كسر قيمة الإرجاع SetFunctionsList حتى عندما تكون الوسائط الخاصة به صالحة.

الصورة 2

أخطاء في التعبيرات الشرطية


V501 هناك تعبيرات فرعية مماثلة إلى اليسار وإلى يمين عامل التشغيل "&&": module && module rootcling_impl.cxx 3650

 virtual void HandleDiagnostic(....) override { .... bool isROOTSystemModuleDiag = module && ....; bool isSystemModuleDiag = module && module && module->IsSystem; if (!isROOTSystemModuleDiag && !isSystemModuleDiag) fChild->HandleDiagnostic(DiagLevel, Info); .... } 

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

V501 هناك تعبيرات فرعية مماثلة 'strchr (fHostAuth-> GetHost () ،' * ')' إلى اليسار وإلى يمين '||' المشغل. TAuthenticate.cxx 300

 TAuthenticate::TAuthenticate(TSocket *sock, const char *remote, const char *proto, const char *user) { .... // If generic THostAuth (ie with wild card or user == any) // make a personalized memory copy of this THostAuth if (strchr(fHostAuth->GetHost(),'*') || strchr(fHostAuth->GetHost(),'*') || fHostAuth->GetServer() == -1 ) { fHostAuth = new THostAuth(*fHostAuth); fHostAuth->SetHost(fqdn); fHostAuth->SetUser(checkUser); fHostAuth->SetServer(servtype); } .... } 

يتم مسح السلسلة fHostAuth-> GetHost () بحثًا عن الحرف '*' مرتين. ربما كان من المفترض أن أحد هذه الشيكات هو البحث عن "؟ الحرف لأن هذين الحرفين هما عادة تلك المستخدمة لتحديد أقنعة البدل المختلفة.

V517 استخدام نمط "if (A) {...} آخر إذا تم اكتشاف (A) {...}". هناك احتمال لوجود خطأ منطقي. خطوط التحقق: 163 ، 165. TProofMonSenderML.cxx 163

 Int_t TProofMonSenderML::SendSummary(TList *recs, const char *id) { .... if (fSummaryVrs == 0) { if ((dsn = recs->FindObject("dataset"))) recs->Remove(dsn); } else if (fSummaryVrs == 0) { // Only the first records xrecs = new TList; xrecs->SetOwner(kFALSE); TIter nxr(recs); TObject *o = 0; while ((o = nxr())) { if (!strcmp(o->GetName(), "vmemmxw")) break; xrecs->Add(o); } } .... } 

تتم مقارنة متغير fSummaryVrs بصفر مرتين ، بحيث لا يصل التنفيذ إلى الكود في الفرع الآخر if . وهناك الكثير من التعليمات البرمجية هناك ...

V523 بيان "ثم" مكافئ لبيان "آخر". TKDTree.cxx 805

 template <typename Index, typename Value> void TKDTree<Index, Value>::UpdateRange(....) { .... if (point[fAxis[inode]]<=fValue[inode]){ //first examine the node that contains the point UpdateRange(GetLeft(inode),point, range, res); UpdateRange(GetRight(inode),point, range, res); } else { UpdateRange(GetLeft(inode),point, range, res); UpdateRange(GetRight(inode),point, range, res); } .... } 

يتم تنفيذ نفس الكتلة من التعليمات البرمجية ، والتي هي استنساخ نسخ لصق ، بغض النظر عن الحالة. أعتقد أن هناك خلط بين الكلمات اليسار واليمين .

المشروع مليء بالمواقع المشبوهة مثل:

  • V523 بيان "ثم" مكافئ لبيان "آخر". TContainerConverters.cxx 51
  • V523 بيان "ثم" مكافئ لبيان "آخر". TWebFile.cxx 1310
  • V523 بيان "ثم" مكافئ لبيان "آخر". MethodMLP.cxx 423
  • V523 بيان "ثم" مكافئ لبيان "آخر". RooAbsCategory.cxx 394

تعبير V547 '! File_name_value.empty ()' غير صحيح دائمًا. SelectionRules.cxx 1423

 bool SelectionRules::AreAllSelectionRulesUsed() const { for(auto&& rule : fClassSelectionRules){ .... std::string file_name_value; if (!rule.GetAttributeValue("file_name", file_name_value)) file_name_value.clear(); if (!file_name_value.empty()) { // <= // don't complain about defined_in rules continue; } const char* attrName = nullptr; const char* attrVal = nullptr; if (!file_name_value.empty()) { // <= attrName = "file name"; attrVal = file_name_value.c_str(); } else { attrName = "class"; if (!name.empty()) attrVal = name.c_str(); } ROOT::TMetaUtils::Warning(0,"Unused %s rule: %s\n", attrName, attrVal); } .... } 

ربما هذا ليس خطأ. المحلل وجدت فقط بعض التعليمات البرمجية التي يمكن تبسيطها. نظرًا لأن قيمة الإرجاع file_name_value.empty () تم التحقق منها بالفعل في بداية الحلقة ، فيمكن إزالة التحقق المكرر الثاني ، وبالتالي التخلص من قدر لا بأس به من التعليمات البرمجية غير الضرورية.

V590 النظر في فحص '! File1 || c <= 0 || ج == '*' || c! = '(' 'التعبير. التعبير مفرط أو يحتوي على خطأ مطبعي. TTabCom.cxx 840

 TString TTabCom::DetermineClass(const char varName[]) { .... c = file1.get(); if (!file1 || c <= 0 || c == '*' || c != '(') { Error("TTabCom::DetermineClass", "variable \"%s\" not defined?", varName); goto cleanup; } .... } 

فيما يلي جزء المشكلة من التعبير الشرطي الذي أبلغ عنه المحلل:

 if (.... || c == '*' || c != '(') { .... } 

لن يؤثر التحقق من حرف العلامة النجمية على نتيجة الحالة. سيكون هذا الجزء صحيحًا دائمًا لأي شخصية بخلاف "(". يمكنك بسهولة التحقق من ذلك بنفسك من خلال رسم جدول الحقيقة.

تحذيران آخران حول الظروف ذات المنطق الغريب:

  • V590 النظر في فحص هذا التعبير. التعبير مفرط أو يحتوي على خطأ مطبعي. TFile.cxx 3963
  • V590 النظر في فحص هذا التعبير. التعبير مفرط أو يحتوي على خطأ مطبعي. TStreamerInfoActions.cxx 3084

V593 خذ بعين الاعتبار مراجعة تعبير النوع 'A = B <C'. يتم حساب التعبير على النحو التالي: 'A = (B <C)'. TProofServ.cxx 1903

 Int_t TProofServ::HandleSocketInput(TMessage *mess, Bool_t all) { .... if (Int_t ret = fProof->AddWorkers(workerList) < 0) { Error("HandleSocketInput:kPROOF_GETSLAVEINFO", "adding a list of worker nodes returned: %d", ret); } .... } 

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

مشكلة أخرى مماثلة:

  • V593 خذ بعين الاعتبار مراجعة تعبير النوع 'A = B <C'. يتم حساب التعبير على النحو التالي: 'A = (B <C)'. TProofServ.cxx 3897

V768 يتم استخدام ثابت التعداد "kCostComplexityPruning" كمتغير من النوع المنطقي. MethodDT.cxx 283
 enum EPruneMethod {kExpectedErrorPruning=0, kCostComplexityPruning, kNoPruning}; void TMVA::MethodDT::ProcessOptions() { .... if (fPruneStrength < 0) fAutomatic = kTRUE; else fAutomatic = kFALSE; if (fAutomatic && fPruneMethod==!DecisionTree::kCostComplexityPruning){ Log() << kFATAL << "Sorry automatic pruning strength determination is ...." << Endl; } .... } 

Hm ... لماذا ينفي kCostComplexityPruning القيمة الثابتة ؟ أظن أن حرف النفي هو خطأ مطبعي ، والذي يشوه الآن منطق التنفيذ.

مؤشر معالجة الأخطاء


V522 قد يتم إلغاء تحديد مؤشر القيمة "pre". TSynapse.cxx 61

 void TSynapse::SetPre(TNeuron * pre) { if (pre) { Error("SetPre","this synapse is already assigned to a pre-neuron."); return; } fpre = pre; pre->AddPost(this); } 

لقد بذلت قصارى جهدي لفهم هذا الرمز الغريب ، ويبدو أن الفكرة كانت لتجنب تعيين قيمة جديدة لحقل fpre . إذا كان الأمر كذلك ، يقوم المبرمج بالتحقق بطريق الخطأ من المؤشر الخاطئ. يؤدي التطبيق الحالي إلى إلغاء مرجع مؤشر فارغ إذا قمت بتمرير قيمة nullptr إلى الدالة SetPre .

أعتقد أنه يجب إصلاح هذا المقتطف كما يلي:

 void TSynapse::SetPre(TNeuron * pre) { if (fpre) { Error("SetPre","this synapse is already assigned to a pre-neuron."); return; } fpre = pre; pre->AddPost(this); } 

ومع ذلك ، لن يمنع هذا تمرير مؤشر فارغ إلى الدالة ، ولكن هذا الإصدار على الأقل أكثر منطقية من الإصدار الأصلي.

يمكن العثور على نسخة معدلة قليلاً من هذا الرمز في مكان آخر:

  • V522 قد تتم عملية إلغاء تأشير المؤشر الخالي. TSynapse.cxx 74

V595 تم استخدام مؤشر 'N' قبل أن يتم التحقق منه ضد nullptr. خطوط التحقق: 484 ، 488. Scanner.cxx 484

 bool RScanner::shouldVisitDecl(clang::NamedDecl *D) { if (auto M = D->getOwningModule()) { // <= 2 return fInterpreter.getSema().isModuleVisible(M); } return true; } bool RScanner::VisitNamespaceDecl(clang::NamespaceDecl* N) { if (fScanType == EScanType::kOnePCM) return true; if (!shouldVisitDecl(N)) // <= 1 return true; if((N && N->isImplicit()) || !N){ // <= 3 return true; } .... } 

هذا هو قطعة خطيرة للغاية من التعليمات البرمجية! لا يتم التحقق من مؤشر N للإلغاء قبل أن يتم إلغاء تسجيله في المرة الأولى. ما هو أكثر من ذلك ، لا يمكنك أن ترى حدوث ذلك هنا لأن التراجع يحدث داخل وظيفة shouldVisitDecl .

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

  • V595 تم استخدام مؤشر 'الملف' قبل أن يتم التحقق منه ضد nullptr. خطوط التحقق: 141 ، 153. TFileCacheRead.cxx 141
  • V595 تم استخدام المؤشر 'fFree' قبل أن يتم التحقق منه ضد nullptr. خطوط التحقق: 2029 ، 2038. TFile.cxx 2029
  • V595 تم استخدام مؤشر 'tbuf' قبل أن يتم التحقق منه ضد nullptr. خطوط التحقق: 586 ، 591. TGText.cxx 586
  • V595 تم استخدام مؤشر 'fPlayer' قبل أن يتم التحقق منه ضد nullptr. خطوط الفحص: 3425 ، 3430. TProof.cxx 3425
  • V595 تم استخدام مؤشر "gProofServ" قبل أن يتم التحقق منه ضد nullptr. خطوط التحقق: 1192 ، 1194. TProofPlayer.cxx 1192
  • V595 تم استخدام مؤشر 'projDataTmp' قبل أن يتم التحقق منه مقابل nullptr. خطوط التحقق: 791 ، 804. RooSimultaneous.cxx 791

التالي ليس خطأ ، لكنه مثال آخر على كيفية تشجيع وحدات الماكرو على كتابة التعليمات البرمجية الخاطئة أو الزائدة عن الحاجة.

الاختيار المتكرر V571 . تم التحقق بالفعل من الشرط "if (fCanvasImp) في السطر 799. TCanvas.cxx 800

 #define SafeDelete(p) { if (p) { delete p; p = 0; } } void TCanvas::Close(Option_t *option) { .... if (fCanvasImp) SafeDelete(fCanvasImp); .... } 

يتم فحص مؤشر fCanvasImp مرتين ، مع إجراء أحد عمليات التحقق المطبقة بالفعل في الماكرو SafeDelete . تتمثل إحدى مشكلات وحدات الماكرو في صعوبة تنقلها من داخل الكود ، وهذا هو السبب وراء عدم قيام العديد من المبرمجين بفحص محتوياتها قبل الاستخدام.

صفيف معالجة الأخطاء


V519 يتم تعيين قيم "Line [Cursor]" مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 352 ، 353. Editor.cpp 353

 size_t find_last_non_alnum(const std::string &str, std::string::size_type index = std::string::npos) { .... char tmp = Line.GetText()[Cursor]; Line[Cursor] = Line[Cursor - 1]; Line[Cursor] = tmp; .... } 

يتم تعيين قيمة جديدة لعنصر السطر [المؤشر] ، ثم يتم استبدالها فورًا. هذا لا يبدو صحيحا ...

V557 تجاوز الصفيف هو ممكن. يشير مؤشر 'ivar' إلى ما وراء مجموعة الصفيف. BasicMinimizer.cxx 130

 bool BasicMinimizer::SetVariableValue(unsigned int ivar, double val) { if (ivar > fValues.size() ) return false; fValues[ivar] = val; return true; } 

ارتكاب هذا الخطأ عند التحقق من فهارس الصفيف هو اتجاه حديث ؛ نراه في كل مشروع ثالث تقريبًا. أثناء الفهرسة في صفيف داخل حلقة سهلة - عادةً ما تستخدم عامل التشغيل "<" للمقارنة بين الفهرس وحجم الصفيف - الشيكات مثل تلك الموضحة أعلاه تتطلب '> =' عامل التشغيل ، وليس '>'. وإلا فإنك تخاطر بفهرسة عنصر واحد خارج حدود الصفيف.

تم استنساخ هذا الخطأ في جميع أنحاء الرمز عدة مرات:

  • V557 تجاوز الصفيف هو ممكن. يشير مؤشر 'ivar' إلى ما وراء مجموعة الصفيف. BasicMinimizer.cxx 186
  • V557 تجاوز الصفيف هو ممكن. يشير مؤشر 'ivar' إلى ما وراء مجموعة الصفيف. BasicMinimizer.cxx 194
  • V557 تجاوز الصفيف هو ممكن. يشير مؤشر 'ivar' إلى ما وراء مجموعة الصفيف. BasicMinimizer.cxx 209
  • V557 تجاوز الصفيف هو ممكن. يشير مؤشر 'ivar' إلى ما وراء مجموعة الصفيف. BasicMinimizer.cxx 215
  • V557 تجاوز الصفيف هو ممكن. يشير مؤشر 'ivar' إلى ما وراء مجموعة الصفيف. BasicMinimizer.cxx 230

V621 فكّر في فحص عامل التشغيل "for". من المحتمل أن يتم تنفيذ الحلقة بشكل غير صحيح أو لن يتم تنفيذها على الإطلاق. TDataMember.cxx 554

 Int_t TDataMember::GetArrayDim() const { if (fArrayDim<0 && fInfo) { R__LOCKGUARD(gInterpreterMutex); TDataMember *dm = const_cast<TDataMember*>(this); dm->fArrayDim = gCling->DataMemberInfo_ArrayDim(fInfo); // fArrayMaxIndex should be zero if (dm->fArrayDim) { dm->fArrayMaxIndex = new Int_t[fArrayDim]; for(Int_t dim = 0; dim < fArrayDim; ++dim) { dm->fArrayMaxIndex[dim] = gCling->DataMemberInfo_MaxIndex(fInfo,dim); } } } return fArrayDim; } 

في الحلقة for ، يبدو أن المطورين قد قصدوا مقارنة المتغير الخافت بـ dm-> fArrayDim بدلاً من fArrayDim . قيمة fArrayDim سالبة ، وهو ما يضمنه الشرط في بداية الوظيفة. وبالتالي ، لن تنفذ هذه الحلقة أبدًا.

V767 وصول مشبوه إلى عنصر من الصفيف "الحالي" بواسطة فهرس ثابت داخل حلقة. TClingUtils.cxx 3082

 llvm::StringRef ROOT::TMetaUtils::DataMemberInfo__ValidArrayIndex(....) { .... while (current!=0) { // Check the token if (isdigit(current[0])) { for(i=0;i<strlen(current);i++) { if (!isdigit(current[0])) { if (errstr) *errstr = current; if (errnum) *errnum = NOT_INT; return llvm::StringRef(); } } } else { // current token is not a digit .... } .... } .... } 

هذا الرمز هو تحليل والتحقق من بعض السلسلة. إذا تم التعرف على الحرف الأول للسلسلة الحالية (أي في الفهرس 0) كرقم ، فستتجاوز الحلقة جميع الأحرف الباقية للتأكد من أن جميعها أرقام. حسنًا ، هذه هي الفكرة على الأقل. المشكلة هي ، لا يتم استخدام العداد i في الحلقة. يجب إعادة كتابة الشرط بحيث يتحقق الحالي [i] بدلاً من الحالي [0] .

صورة 4

تسرب الذاكرة


V773 تم إنهاء الوظيفة دون تحرير مؤشر 'optionlist'. تسرب الذاكرة ممكن. TDataMember.cxx 355

 void TDataMember::Init(bool afterReading) { .... TList *optionlist = new TList(); //storage for options strings for (i=0;i<token_cnt;i++) { if (strstr(tokens[i],"Items")) { ptr1 = R__STRTOK_R(tokens[i], "()", &rest); if (ptr1 == 0) { Fatal("TDataMember","Internal error, found \"Items....",GetTitle()); return; } ptr1 = R__STRTOK_R(nullptr, "()", &rest); if (ptr1 == 0) { Fatal("TDataMember","Internal error, found \"Items....",GetTitle()); return; } .... } .... } .... // dispose of temporary option list... delete optionlist; .... } 

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

memset مرة أخرى


V597 قد يقوم المحول البرمجي بحذف استدعاء دالة 'memset' ، والذي يُستخدم في مسح المخزن المؤقت 'x'. يجب استخدام الدالة memset_s () لمسح البيانات الخاصة. TMD5.cxx 366

 void TMD5::Transform(UInt_t buf[4], const UChar_t in[64]) { UInt_t a, b, c, d, x[16]; .... // Zero out sensitive information memset(x, 0, sizeof(x)); } 

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

متنوع


V591 يجب أن ترجع الدالة Non-void قيمة. LogLikelihoodFCN.h 108

 LogLikelihoodFCN & operator = (const LogLikelihoodFCN & rhs) { SetData(rhs.DataPtr() ); SetModelFunction(rhs.ModelFunctionPtr() ); fNEffPoints = rhs.fNEffPoints; fGrad = rhs.fGrad; fIsExtended = rhs.fIsExtended; fWeight = rhs.fWeight; fExecutionPolicy = rhs.fExecutionPolicy; } 

المشغل المثقل لا يوجد لديه قيمة الإرجاع. هذا هو الاتجاه الأخير في الآونة الأخيرة.

V596 تم تكوين العنصر ولكن لا يتم استخدامه. قد تكون الكلمة المفتاحية "رمي" مفقودة: رمي وقت التشغيل (FOO) ؛ RTensor.hxx 363

 template <typename Value_t, typename Container_t> inline RTensor<Value_t, Container_t> RTensor<Value_t, Container_t>::Transpose() { if (fLayout == MemoryLayout::RowMajor) { fLayout = MemoryLayout::ColumnMajor; } else if (fLayout == MemoryLayout::ColumnMajor) { fLayout = MemoryLayout::RowMajor; } else { std::runtime_error("Memory layout is not known."); } .... } 

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

لم يكن هناك سوى تحذيرين من هذا النوع. ها هي الثانية:

  • V596 تم تكوين العنصر ولكن لا يتم استخدامه. قد تكون الكلمة المفتاحية "رمي" مفقودة: رمي وقت التشغيل (FOO) ؛ Forest.hxx 137

V609 قسّم على صفر. نطاق المقام [0..100]. TGHtmlImage.cxx 340

 const char *TGHtml::GetPctWidth(TGHtmlElement *p, char *opt, char *ret) { int n, m, val; .... if (n < 0 || n > 100) return z; if (opt[0] == 'h') { val = fCanvas->GetHeight() * 100; } else { val = fCanvas->GetWidth() * 100; } if (!fInTd) { snprintf(ret, 15, "%d", val / n); // <= } else { .... } .... } 

يشبه هذا المثال معالجة الصفيف التي تمت مناقشتها مسبقًا. يقتصر المتغير n على النطاق من 0 إلى 100. ولكن بعد ذلك ، يوجد فرع يقوم بالقسمة على المتغير n والذي قد يكون له القيمة 0. وأعتقد أن حدود النطاق n يجب أن تكون ثابتة كما يلي:

 if (n <= 0 || n > 100) return z; 

V646 النظر في فحص منطق التطبيق. من المحتمل أن الكلمة الرئيسية "أخرى" مفقودة. TProofServ.cxx 729

 TProofServ::TProofServ(Int_t *argc, char **argv, FILE *flog) : TApplication("proofserv", argc, argv, 0, -1) { .... if (!logmx.IsDigit()) { if (logmx.EndsWith("K")) { xf = 1024; logmx.Remove(TString::kTrailing, 'K'); } else if (logmx.EndsWith("M")) { xf = 1024*1024; logmx.Remove(TString::kTrailing, 'M'); } if (logmx.EndsWith("G")) { xf = 1024*1024*1024; logmx.Remove(TString::kTrailing, 'G'); } } .... } 

يقوم المحلل بالإبلاغ عن عبارة if منسقة بشكل غريب إذا كانت الكلمة الرئيسية مفقودة. الطريقة التي يبدو بها هذا الرمز تشير إلى أنه يحتاج إلى إصلاح.

تحذيرات أخرى من هذا النوع:

  • V646 النظر في فحص منطق التطبيق. من المحتمل أن الكلمة الرئيسية "أخرى" مفقودة. TFormula_v5.cxx 3702
  • V646 النظر في فحص منطق التطبيق. من المحتمل أن الكلمة الرئيسية "أخرى" مفقودة. RooAbsCategory.cxx 604

حلقة V663 اللانهائية ممكنة. شرط "cin.eof ()" غير كاف للكسر من الحلقة. ضع في اعتبارك إضافة استدعاء دالة 'cin.fail () إلى التعبير الشرطي. MethodKNN.cxx 602

 void TMVA::MethodKNN::ReadWeightsFromStream(std::istream& is) { .... while (!is.eof()) { std::string line; std::getline(is, line); if (line.empty() || line.find("#") != std::string::npos) { continue; } .... } .... } 

عند العمل مع فئة std :: istream ، فإن استدعاء وظيفة eof () لا يكفي لإنهاء الحلقة. ستُرجع الدالة eof () دائمًا خطأ إذا لم تتم قراءة البيانات ، ولم تكن هناك نقاط إنهاء أخرى في هذا الرمز. لضمان إنهاء الحلقة ، يتطلب الأمر إجراء فحص إضافي للقيمة التي يتم إرجاعها بواسطة الدالة fail () :

 while (!is.eof() && !is.fail()) { .... } 

كبديل ، يمكن إعادة كتابته على النحو التالي:

 while (is) { .... } 

V678 يتم استخدام العنصر كوسيطة للأسلوب الخاص به. فكر في التحقق من الوسيطة الفعلية الأولى لوظيفة "نسخ". TFormLeafInfo.cxx 2414

 TFormLeafInfoMultiVarDim::TFormLeafInfoMultiVarDim( const TFormLeafInfoMultiVarDim& orig) : TFormLeafInfo(orig) { fNsize = orig.fNsize; fSizes.Copy(fSizes); // <= fCounter2 = orig.fCounter2?orig.fCounter2->DeepCopy():0; fSumOfSizes = orig.fSumOfSizes; fDim = orig.fDim; fVirtDim = orig.fVirtDim; fPrimaryIndex = orig.fPrimaryIndex; fSecondaryIndex = orig.fSecondaryIndex; } 

دعنا ننهي المقالة بهذا الخطأ الإملائي الصغير. يجب استدعاء وظيفة النسخ بـ orig.fSizes ، وليس fSizes .

استنتاج


منذ حوالي عام ، فحصنا مشروع NCBI Genome Workbench ، وهو برنامج آخر يستخدم في البحث العلمي ويتناول تحليل الجينوم. أذكر ذلك لأن جودة البرامج العلمية أمر بالغ الأهمية ، لكن المطورين يميلون إلى التقليل من أهمية ذلك.

بالمناسبة ، تم إصدار macOS 10.15 Catalina في اليوم الآخر ، حيث توقفوا عن دعم تطبيقات 32 بت. لحسن الحظ ، يقدم PVS-Studio مجموعة كبيرة من التشخيصات المصممة خصيصًا للكشف عن الأخطاء التي تصاحب نقل البرامج إلى أنظمة 64 بت. تعرف على المزيد في هذا المنشور بواسطة فريق PVS-Studio.

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


All Articles