Geekly articles weekly

CMake：当项目不可原谅时，其代码质量

图片1

CMake是用于建筑项目的跨平台自动化系统。 该系统比PVS-Studio静态代码分析器要古老得多，而还没有人试图将其应用于代码并检查错误。 原来有很多错误。 CMake的观众很多。 在此，新项目开始，旧项目被转移。 很难想象有多少程序员可能遇到这个或那个错误。

引言


CMake （来自英语的跨平台制造商）是一种跨平台自动化系统，用于从源代码构建软件。 CMake不直接构建，而仅从CMakeLists.txt文件生成构建控制文件。 该程序的第一版于2000年发布。 为了进行比较， PVS-Studio静态分析仪仅在2008年出现。 然后，它专注于在将程序从32位系统移植到64位系统时发现错误，并在2010年出现了第一套通用诊断程序（ V501 - V545 ）。 顺便说一句，从CMake代码的第一个设置中有一些警告。

不可原谅的错误


V1040预定义的宏名称的拼写可能有错字。 “ __MINGW32_”宏类似于“ __MINGW32__”。 winapi.h 4112

/* from winternl.h */ #if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_) #define __UNICODE_STRING_DEFINED #endif

V1040诊断仅在最近才实施。 在本文发表时，很可能不会发布该文章，但是借助此诊断程序，我们已经找到了一个棘手的错误。

他们在这里以__MINGW32_的名称打错了字 。 最后，缺少一个下划线。 如果使用具有该名称的代码进行搜索，则可以确保项目实际使用的版本在两侧均带有两个下划线：

图片3


V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 558

 bool IsVisualStudioMacrosFileRegistered(const std::string& macrosFile, const std::string& regKeyBase, std::string& nextAvailableSubKeyName) { .... if (ERROR_SUCCESS == result) { wchar_t subkeyname[256]; // <= DWORD cch_subkeyname = sizeof(subkeyname) * sizeof(subkeyname[0]); // <= wchar_t keyclass[256]; DWORD cch_keyclass = sizeof(keyclass) * sizeof(keyclass[0]); FILETIME lastWriteTime; lastWriteTime.dwHighDateTime = 0; lastWriteTime.dwLowDateTime = 0; while (ERROR_SUCCESS == RegEnumKeyExW(hkey, index, subkeyname, &cch_subkeyname, 0, keyclass, &cch_keyclass, &lastWriteTime)) { .... } .... }

静态声明数组时， sizeof运算符将同时考虑元素的数量和元素的大小，以字节为单位计算其大小。 在计算变量cch_subkeyname的值时，程序员没有考虑到这一点，而是收到了比计划大4倍的值。 让我们解释一下“ 4次”。

数组及其不正确的大小将传递给RegEnumKeyExW函数：

 LSTATUS RegEnumKeyExW( HKEY hKey, DWORD dwIndex, LPWSTR lpName, // <= subkeyname LPDWORD lpcchName, // <= cch_subkeyname LPDWORD lpReserved, LPWSTR lpClass, LPDWORD lpcchClass, PFILETIME lpftLastWriteTime );

lpcchName指针必须指向包含以字符指定的缓冲区大小的变量：“指向以lpClass参数指定的缓冲区大小（以字符为单位）的变量的指针”。 子键名数组的大小为512个字节，并且能够存储256个wchar_t类型的字符（在Windows wchar_t中为2个字节）。 此值为256，应将其传递给函数。 取而代之的是，将512再乘以2得到1024。

我认为现在已经知道如何解决该错误。 代替乘法，使用除法：

 DWORD cch_subkeyname = sizeof(subkeyname) / sizeof(subkeyname[0]);

顺便说一句，计算cch_keyclass变量的值时会发生完全相同的错误。

所描述的错误有可能导致缓冲区溢出。 有必要修复所有这些地方：

  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 556
  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 572
  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 621
  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 622
  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 649

V595在针对nullptr进行验证之前，已使用“ this-> BuildFileStream”指针。 检查行：133，134。cmMakefileTargetGenerator.cxx 133

 void cmMakefileTargetGenerator::CreateRuleFile() { .... this->BuildFileStream->SetCopyIfDifferent(true); if (!this->BuildFileStream) { return; } .... }

在验证检查之前， this-> BuildFileStream指针被取消引用。 这真的引起任何问题吗？ 下面是这样一个地方的另一个例子。 它就在复写纸下面。 但是实际上， 有很多V595警告，而且大多数警告并不那么明显。 根据经验，我可以说纠正这种诊断的警告最长。

  • V595在针对nullptr对其进行验证之前，已使用“ this-> FlagFileStream”指针。 检查行：303、304。cmMakefileTargetGenerator.cxx 303

V614使用了未初始化的指针'str'。 cmVSSetupHelper.h 80

 class SmartBSTR { public: SmartBSTR() { str = NULL; } SmartBSTR(const SmartBSTR& src) { if (src.str != NULL) { str = ::SysAllocStringByteLen((char*)str, ::SysStringByteLen(str)); } else { str = ::SysAllocStringByteLen(NULL, 0); } } .... private: BSTR str; };

分析器检测到未初始化指针str的使用 。 这是由于通常的错字而引起的。 调用SysAllocStringByteLen函数时，必须使用src.str指针。

V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2749

 static int64_t expand(struct archive_read *a, int64_t end) { .... if ((lensymbol = read_next_symbol(a, &rar->lengthcode)) < 0) goto bad_data; if (lensymbol > (int)(sizeof(lengthbases)/sizeof(lengthbases[0]))) goto bad_data; if (lensymbol > (int)(sizeof(lengthbits)/sizeof(lengthbits[0]))) goto bad_data; len = lengthbases[lensymbol] + 2; if (lengthbits[lensymbol] > 0) { if (!rar_br_read_ahead(a, br, lengthbits[lensymbol])) goto truncated_data; len += rar_br_bits(br, lengthbits[lensymbol]); rar_br_consume(br, lengthbits[lensymbol]); } .... }

在这段代码中发现了几个问题。 当访问lengthbases和lengthbits数组时，可能会超出数组的边界，因为在代码上方，开发人员将运算符写为'>'而不是'> ='。 这样的检查开始跳过一个无效值。 我们面临着一种经典的错误模式，称为Off-by-one Error 。

无效索引访问数组的整个位置列表：

  • V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2750
  • V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2751
  • V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2753
  • V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2754
  • V557阵列可能超限。 “ offssymbol”索引的值可能达到60。archive_read_support_format_rar.c 2797

内存泄漏


V773在不释放'testRun'指针的情况下退出了该函数。 可能发生内存泄漏。 cmCTestMultiProcessHandler.cxx 193

 void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner, bool started) { .... delete runner; if (started) { this->StartNextTests(); } } bool cmCTestMultiProcessHandler::StartTestProcess(int test) { .... cmCTestRunTest* testRun = new cmCTestRunTest(*this); // <= .... if (testRun->StartTest(this->Completed, this->Total)) { return true; // <= } this->FinishTestProcess(testRun, false); // <= return false; }

分析仪检测到内存泄漏。 如果函数testRun-> StartTest返回true， 则不会释放指针testRun的内存。 当执行另一个代码分支时，使用testRun指针的内存在this-> FinishTestProcess函数中释放。

资源泄漏


V773在没有关闭'fd'句柄引用的文件的情况下退出了该函数。 资源泄漏是可能的。 rhash.c 450

 RHASH_API int rhash_file(....) { FILE* fd; rhash ctx; int res; hash_id &= RHASH_ALL_HASHES; if (hash_id == 0) { errno = EINVAL; return -1; } if ((fd = fopen(filepath, "rb")) == NULL) return -1; if ((ctx = rhash_init(hash_id)) == NULL) return -1; // <= fclose(fd); ??? res = rhash_file_update(ctx, fd); fclose(fd); rhash_final(ctx, result); rhash_free(ctx); return res; }

条件下的奇怪逻辑


V590考虑检查'* s！='\ 0'&& * s =='''表达式。 表达式过多或打印错误。 archive_cmdline.c 76

 static ssize_t get_argument(struct archive_string *as, const char *p) { const char *s = p; archive_string_empty(as); /* Skip beginning space characters. */ while (*s != '\0' && *s == ' ') s++; .... }

将* s字符与零终止符进行比较是多余的。 while循环的条件仅取决于字符是否等于空格。 这不是错误，而是代码的额外复杂性。

V592该表达式用括号括起来两次：（（expression））。 不需要一对括号，否则会出现打印错误。 cmCTestTestHandler.cxx 899

 void cmCTestTestHandler::ComputeTestListForRerunFailed() { this->ExpandTestsToRunInformationForRerunFailed(); ListOfTests finalList; int cnt = 0; for (cmCTestTestProperties& tp : this->TestList) { cnt++; // if this test is not in our list of tests to run, then skip it. if ((!this->TestsToRun.empty() && std::find(this->TestsToRun.begin(), this->TestsToRun.end(), cnt) == this->TestsToRun.end())) { continue; } tp.Index = cnt; finalList.push_back(tp); } .... }

分析仪警告说，也许应该将否定放在方括号内。 似乎这里没有这样的错误-只是多余的双括号。 但是，很可能在这种情况下存在逻辑错误。

如果this-> TestsToRun的测试列表不为空并且其中不存在cnt ，则将执行continue语句。 逻辑上假设，如果测试列表为空，则必须执行相同的操作。 最可能的情况是这样的：

 if (this->TestsToRun.empty() || std::find(this->TestsToRun.begin(), this->TestsToRun.end(), cnt) == this->TestsToRun.end()) { continue; }

V592该表达式用括号括起来两次：（（expression））。 不需要一对括号，否则会出现打印错误。 cmMessageCommand.cxx 73

 bool cmMessageCommand::InitialPass(std::vector<std::string> const& args, cmExecutionStatus&) { .... } else if (*i == "DEPRECATION") { if (this->Makefile->IsOn("CMAKE_ERROR_DEPRECATED")) { fatal = true; type = MessageType::DEPRECATION_ERROR; level = cmake::LogLevel::LOG_ERROR; } else if ((!this->Makefile->IsSet("CMAKE_WARN_DEPRECATED") || this->Makefile->IsOn("CMAKE_WARN_DEPRECATED"))) { type = MessageType::DEPRECATION_WARNING; level = cmake::LogLevel::LOG_WARNING; } else { return true; } ++i; } .... }

类似的例子，但是在这里我对出现错误更有信心。 IsSet函数（“ CMAKE_WARN_DEPRECATED”）验证是否已全局设置CMAKE_WARN_DEPRECATED值，并且IsOn函数（“ CMAKE_WARN_DEPRECATED”）检查该值是否在项目配置中指定。 否定运算符很可能是多余的，因为 在这两种情况下，设置相同的类型和级别值都是正确的。

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

 bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started) { .... } else if ((success && !this->TestProperties->WillFail) || (!success && this->TestProperties->WillFail)) { this->TestResult.Status = cmCTestTestHandler::COMPLETED; outputStream << " Passed "; } .... }

通过这样重写条件表达式，可以大大简化此类代码：

 } else if (success != this->TestProperties->WillFail) { this->TestResult.Status = cmCTestTestHandler::COMPLETED; outputStream << " Passed "; }

您可以简化一些其他地方：

  • V728过度检查可以简化。 '（A && B）|| （！A &&！B）'表达式等同于'bool（A）== bool（B）'表达式。 cmCTestTestHandler.cxx 702
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 摘要_sspi.c 443
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 tcp.c 1295
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 testDynamicLoader.cxx 58
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 testDynamicLoader.cxx 65
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 testDynamicLoader.cxx 72

杂项警告


V523'then '语句等效于后续代码片段。 archive_read_support_format_ar.c 415

 static int _ar_read_header(struct archive_read *a, struct archive_entry *entry, struct ar *ar, const char *h, size_t *unconsumed) { .... /* * "__.SYMDEF" is a BSD archive symbol table. */ if (strcmp(filename, "__.SYMDEF") == 0) { archive_entry_copy_pathname(entry, filename); /* Parse the time, owner, mode, size fields. */ return (ar_parse_common_header(ar, entry, h)); } /* * Otherwise, this is a standard entry. The filename * has already been trimmed as much as possible, based * on our current knowledge of the format. */ archive_entry_copy_pathname(entry, filename); return (ar_parse_common_header(ar, entry, h)); }

最后条件中的表达式与函数的后两行相同。 可以通过删除条件来简化此代码，或者代码中有错误，应予以修复。

V535变量“ i”用于该循环和外部循环。 检查线：2220、2241。multi.c 2241

 static CURLMcode singlesocket(struct Curl_multi *multi, struct Curl_easy *data) { .... for(i = 0; (i< MAX_SOCKSPEREASYHANDLE) && // <= (curraction & (GETSOCK_READSOCK(i) | GETSOCK_WRITESOCK(i))); i++) { unsigned int action = CURL_POLL_NONE; unsigned int prevaction = 0; unsigned int comboaction; bool sincebefore = FALSE; s = socks[i]; /* get it from the hash */ entry = sh_getentry(&multi->sockhash, s); if(curraction & GETSOCK_READSOCK(i)) action |= CURL_POLL_IN; if(curraction & GETSOCK_WRITESOCK(i)) action |= CURL_POLL_OUT; actions[i] = action; if(entry) { /* check if new for this transfer */ for(i = 0; i< data->numsocks; i++) { // <= if(s == data->sockets[i]) { prevaction = data->actions[i]; sincebefore = TRUE; break; } } } .... }

变量i用作外部循环和嵌套循环中的循环计数器。 在这种情况下，计数器值再次从封闭的1中的0开始计数。 在这里这可能不是一个错误，但是代码很可疑。

V519'tagString '变量被连续两次赋值。 也许这是一个错误。 检查行：84，86。cmCPackLog.cxx 86

 oid cmCPackLog::Log(int tag, const char* file, int line, const char* msg, size_t length) { .... if (tag & LOG_OUTPUT) { output = true; display = true; if (needTagString) { if (!tagString.empty()) { tagString += ","; } tagString = "VERBOSE"; } } if (tag & LOG_WARNING) { warning = true; display = true; if (needTagString) { if (!tagString.empty()) { tagString += ","; } tagString = "WARNING"; } } .... }

在所有位置，新值都会使tagString变量磨损 。 很难说出错误是什么，或者为什么会这样。 也许运算符'='和'+ ='感到困惑。

这些地方的整体清单：

  • V519'tagString'变量被连续两次赋值。 也许这是一个错误。 检查行：94，96。cmCPackLog.cxx 96
  • V519'tagString'变量被连续两次赋值。 也许这是一个错误。 检查行：104，106。cmCPackLog.cxx 106
  • V519'tagString'变量被连续两次赋值。 也许这是一个错误。 检查行：114，116。cmCPackLog.cxx 116
  • V519'tagString'变量被连续两次赋值。 也许这是一个错误。 检查行：125、127。cmCPackLog.cxx 127

V519连续两次给' aes- > aes_set'变量赋值。 也许这是一个错误。 检查行：4052、4054。archive_string.c 4054

 int archive_mstring_copy_utf8(struct archive_mstring *aes, const char *utf8) { if (utf8 == NULL) { aes->aes_set = 0; // <= } aes->aes_set = AES_SET_UTF8; // <= .... return (int)strlen(utf8); }

强制AES_SET_UTF8看起来可疑。 我认为这样的代码会误导任何面对这个地方进行改进的开发人员。

这段代码被复制到另一个地方：

  • V519连续两次给'aes-> aes_set'变量赋值。 也许这是一个错误。 检查行：4066，4068。archive_string.c 4068

如何在CMake上的项目中发现错误


在本节中，我将向您介绍如何使用PVS-Studio轻松地在CMake上检查项目。

Windows / Visual Studio

对于Visual Studio，可以使用CMake GUI或以下命令生成项目文件：

 cmake -G "Visual Studio 15 2017 Win64" ..

接下来，您可以打开.sln文件，并使用Visual Studio 插件测试项目。

Linux / macOS

在这些系统上，compile_commands.json文件用于验证项目。 顺便说一下，它可以在不同的组装系统中生成。 在CMake中，这是这样完成的：

 cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On ..

仍然需要使用.json文件在目录中启动分析器：

 pvs-studio-analyzer analyze -l /path/to/PVS-Studio.lic -o /path/to/project.log -e /path/to/exclude-path -j<N>

我们还为CMake项目开发了一个模块。 有些人喜欢使用它。 CMake模块及其使用示例可以在我们在GitHub上的存储库中找到： pvs-studio-cmake-examples 。

结论


CMake用户的庞大受众是该项目的良好测试者，但是使用PVS-Studio之类的静态代码分析工具，在发行之前无法避免许多问题。

如果您喜欢分析器的结果，但是您的项目不是用C和C ++编写的，那么我想提醒您，分析器还支持使用C＃和Java分析项目。 您可以转到此页面在项目上试用分析仪。



如果您想与说英语的读者分享这篇文章，请使用以下链接：Svyatoslav Razmyslov。 CMake：项目质量不可原谅的情况 。

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

More articles:


All Articles