Geekly articles weekly

适用于C和C ++的最佳复制粘贴算法。 Haiku OS食谱

大量的错别字和复制粘贴代码成为有关通过PVS-Studio分析仪检查Haiku代码的其他文章的主题。 然而，本文主要讲述的是与漫不经心和重构失败有关的错误，而不是错别字。 发现的错误表明人为因素在软件开发中的强大程度。

图片1

引言


Haiku是用于个人计算机的免费开源操作系统。 一个国际开发团队目前正在研究系统的组件。 在最新的重大开发改进中，在操作系统中移植Libre Office和第一个R1 Beta 1版本十分引人注目。

自2015年以来， PVS-Studio的开发人员团队一直关注该项目的开发，并发布了代码缺陷的评论。 这是有史以来的第四次评论。 您可以通过以下链接阅读以前的文章：

  1. 通过PVS-Studio，分析Haiku操作系统（BeOS系列），第1部分 ；
  2. 通过PVS-Studio分析Haiku操作系统（BeOS系列）。 第二部分 ;
  3. 如何在C和C ++中放纵自己。 Haiku OS食谱

最后的代码分析的功能是可以使用Linux的PVS-Studio正式版。 2015年既没有提供适用于Linux的PVS-Studio，也没有提供用于查看错误的便捷报告。这次，我们将以便捷的格式将完整报告发送给Haiku开发人员。

经典版


V501'-'运算符的左侧和右侧有相同的子表达式：（addr_t）b-（addr_t）b BitmapManager.cpp 51

int compare_app_pointer(const ServerApp* a, const ServerApp* b) { return (addr_t)b - (addr_t)b; }

每个开发人员一生必须至少混合一次变量a和b ， x和y ， i和j ...。

V501在'||'的左侧和右侧有相同的子表达式 运算符：输入== __null || 输入== __null MediaClient.cpp 182

 status_t BMediaClient::Unbind(BMediaInput* input, BMediaOutput* output) { CALLED(); if (input == NULL || input == NULL) return B_ERROR; if (input->fOwner != this || output->fOwner != this) return B_ERROR; input->fBind = NULL; output->fBind = NULL; return B_OK; }

在条件中两次检查同一输入指针。 而输出指针保持未检查状态，这可能导致空指针取消引用。

固定代码：

 if (input == NULL || output == NULL) return B_ERROR;

V583 ' ？： '运算符，无论其条件表达式如何，始终返回一个相同的值：500000。usb_modeswitch.cpp 361
 static status_t my_transfer_data(....) { .... do { bigtime_t timeout = directionIn ? 500000 : 500000; result = acquire_sem_etc(device->notify, 1, B_RELATIVE_TIMEOUT, timeout); .... } while (result == B_INTERRUPTED); .... }

当代码编写者犯错并写了两个相同的返回值500000时，三元运算符变得毫无意义。

V519连续两次给'm_kindex1'变量赋值两次。 也许这是一个错误。 检查行：40，41。agg_trans_double_path.cpp 41

 trans_double_path::trans_double_path() : m_kindex1(0.0), m_kindex2(0.0), m_base_length(0.0), m_base_height(1.0), m_status1(initial), m_status2(initial), m_preserve_x_scale(true) { } void trans_double_path::reset() { m_src_vertices1.remove_all(); m_src_vertices2.remove_all(); m_kindex1 = 0.0; m_kindex1 = 0.0; m_status1 = initial; m_status2 = initial; }

重置函数中有一个错误： m_kindex2变量索引中的错字。 该变量不会被重置，这可能会影响其他代码片段的执行。

V501在'>'运算符的左侧和右侧有相同的子表达式：fg [order_type :: R]> fg [order_type :: R] agg_span_image_filter_rgba.h 898

 typedef Source source_type; typedef typename source_type::color_type color_type; typedef typename source_type::order_type order_type; void generate(color_type* span, int x, int y, unsigned len) { .... if(fg[0] < 0) fg[0] = 0; if(fg[1] < 0) fg[1] = 0; if(fg[2] < 0) fg[2] = 0; if(fg[3] < 0) fg[3] = 0; if(fg[order_type::A] > base_mask) fg[order_type::A] = base_mask; if(fg[order_type::R] > fg[order_type::R])fg[order_type::R] = fg[order_type::R]; if(fg[order_type::G] > fg[order_type::G])fg[order_type::G] = fg[order_type::G]; if(fg[order_type::B] > fg[order_type::B])fg[order_type::B] = fg[order_type::B]; .... }

在最后几行中，同时存在两个问题：比较和相等变量的分配。 我什至无法暗示作者的想法是什么。 我只是将这个片段记为可疑。

V570'wPipeIndex'变量被分配给它自己。 CEchoGals_transport.cpp 244

 ECHOSTATUS CEchoGals::CloseAudio (....) { .... wPipeIndex = wPipeIndex; m_ProcessId[ wPipeIndex ] = NULL; m_Pipes[ wPipeIndex ].wInterleave = 0; .... }

wPipeIndex变量由其自己的值初始化。 很可能是打错了字。

指针错误


V522可能会取消引用空指针“ currentInterface”。 Device.cpp 258

 Device::Device(....) : .... { .... usb_interface_info* currentInterface = NULL; // <= uint32 descriptorStart = sizeof(usb_configuration_descriptor); while (descriptorStart < actualLength) { switch (configData[descriptorStart + 1]) { .... case USB_DESCRIPTOR_ENDPOINT: { .... if (currentInterface == NULL) // <= break; currentInterface->endpoint_count++; .... } .... case USB_DESCRIPTOR_ENDPOINT_COMPANION: { usb_endpoint_descriptor* desc = currentInterface // <= ->endpoint[currentInterface->endpoint_count - 1].descr; .... } .... }

currentInterface指针最初由null初始化，然后在进入switch运算符的分支中时进行检查，但并非在所有情况下均如此。 分析器警告说，当跳转到USB_DESCRIPTOR_ENDPOINT_COMPANION案例标签时，可能会发生空指针取消引用。

V522可能会取消引用空指针“目录”。 PathMonitor.cpp 1465

 bool PathHandler::_EntryCreated(....) { .... Directory* directory = directoryNode->ToDirectory(); if (directory == NULL) { // We're out of sync with reality. if (!dryRun) { if (Entry* nodeEntry = directory->FirstNodeEntry()) { .... } } return false; } .... }

我认为目录指针与null值的比较条件存在错误； 条件必须相反。 在当前实现中，如果dryRun变量为false ，则将取消引用目录空指针。

V522可能会取消引用空指针“输入”。 MediaRecorder.cpp 343

 void GetInput(media_input* input); const media_input& BMediaRecorder::MediaInput() const { CALLED(); media_input* input = NULL; fNode->GetInput(input); return *input; }

输入指针由null初始化，并保持该值不变，因为该指针在GetInput函数中未更改。 在BMediaRecorder类的其他方法中，实现是不同的，例如：

 status_t BMediaRecorder::_Connect(....) { .... // Find our Node's free input media_input ourInput; fNode->GetInput(&ourInput); // <= .... }

在这里都是正确的，但是必须重写第一个片段，否则该函数将返回对本地对象的引用。

V522可能会取消引用空指针“ mustFree”。 RequestUnflattener.cpp 35

 status_t Reader::Read(int32 size, void** buffer, bool* mustFree) { if (size < 0 || !buffer || mustFree) // <= return B_BAD_VALUE; if (size == 0) { *buffer = NULL; *mustFree = false; // <= return B_OK; } .... }

在检查所有不正确数据的条件表达式中，作者在检查mustFree指针时输入了错误。 最有可能的是，当该指针的值为null时，该函数应退出：

 if (size < 0 || !buffer || !mustFree) // <= return B_BAD_VALUE;

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

 void checkStructure(Disk &disk) { .... Inode* missing = gMissing.Get(run); dir = dynamic_cast<Directory *>(missing); if (missing == NULL) { .... } .... }

开发人员应该检查dir指针，而不要在类型转换后丢失 。 顺便说一下，C＃开发人员也经常犯类似的错误。 这再次证明了某些错误不取决于所使用的语言。

代码中还有两个类似的地方：

  • V757使用'dynamic_cast'进行类型转换后，可能会将不正确的变量与nullptr进行比较。 检查行：355，357。ExpandoMenuBar.cpp 355
  • V757使用'dynamic_cast'进行类型转换后，可能会将不正确的变量与nullptr进行比较。 检查行：600，601。ValControl.cpp 600

索引错误


V557阵列可能超限。 “ BT_SCO”索引指向数组边界之外。 h2upper.cpp 75

 struct bt_usb_dev { .... struct list nbuffersTx[(1 + 1 + 0 + 0)]; // <= [0..1] .... } typedef enum { BT_COMMAND = 0, BT_EVENT, BT_ACL, BT_SCO, // <= 3 BT_ESCO, HCI_NUM_PACKET_TYPES } bt_packet_t; void sched_tx_processing(bt_usb_dev* bdev) { .... if (!list_is_empty(&bdev->nbuffersTx[BT_SCO])) { // <= fail // TODO to be implemented } .... }

bdev-> nbuffersTx数组仅包含2个元素，但是它是由BT_SCO常量3寻址的。这是surefire数组索引的范围。

V557阵列可能超限。 “ ieee80211_send_setup”功能处理值“ 16”。 检查第四个参数。 检查线：842、911。ieee80211_output.c 842

 struct ieee80211_node { .... struct ieee80211_tx_ampdu ni_tx_ampdu[16]; // <= [0..15] .... }; #define IEEE80211_NONQOS_TID 16 int ieee80211_mgmt_output(....) { .... ieee80211_send_setup(ni, m, IEEE80211_FC0_TYPE_MGT | type, IEEE80211_NONQOS_TID, // <= 16 vap->iv_myaddr, ni->ni_macaddr, ni->ni_bssid); .... } void ieee80211_send_setup( struct ieee80211_node *ni, struct mbuf *m, int type, int tid, // <= 16 ....) { .... tap = &ni->ni_tx_ampdu[tid]; // <= 16 .... }

另一个数组索引超出范围。 这次，只增加了一个要素。 进程间分析有助于揭示由索引16寻址由16个元素组成的ni-> ni_tx_ampdu数组的情况。在C和C ++数组中，索引从零开始。

V781使用“ vector”变量后，将对其进行检查。 程序逻辑中可能有一个错误。 检查行：802、805。oce_if.c 802

 #define OCE_MAX_EQ 32 typedef struct oce_softc { .... OCE_INTR_INFO intrs[OCE_MAX_EQ]; .... } OCE_SOFTC, *POCE_SOFTC; static int oce_alloc_intr(POCE_SOFTC sc, int vector, void (*isr) (void *arg, int pending)) { POCE_INTR_INFO ii = &sc->intrs[vector]; int rc = 0, rr; if (vector >= OCE_MAX_EQ) return (EINVAL); .... }

分析器检测到sc-> intrs数组的元素被无效索引寻址，该索引超出范围。 原因是代码中的操作顺序不正确。 首先，对元素进行寻址，然后检查索引值是否有效。

有人可能会说不会有任何麻烦。 它不会删除array元素的值，而只是获取单元格的地址。 但是，这不是做事情的方式。 阅读更多：“ 空指针取消引用导致未定义的行为 ”。

V519该变量连续两次被赋值。 也许这是一个错误。 检查行：199，200。nvme_ctrlr.c 200

 static void nvme_ctrlr_set_intel_supported_features(struct nvme_ctrlr *ctrlr) { bool *supported_feature = ctrlr->feature_supported; supported_feature[NVME_INTEL_FEAT_MAX_LBA] = true; supported_feature[NVME_INTEL_FEAT_MAX_LBA] = true; supported_feature[NVME_INTEL_FEAT_NATIVE_MAX_LBA] = true; supported_feature[NVME_INTEL_FEAT_POWER_GOVERNOR_SETTING] = true; supported_feature[NVME_INTEL_FEAT_SMBUS_ADDRESS] = true; supported_feature[NVME_INTEL_FEAT_LED_PATTERN] = true; supported_feature[NVME_INTEL_FEAT_RESET_TIMED_WORKLOAD_COUNTERS] = true; supported_feature[NVME_INTEL_FEAT_LATENCY_TRACKING] = true; }

索引为NVME_INTEL_FEAT_MAX_LBA的数组元素分配了相同的值。 好消息是此函数提供了所有可能的常量，这使得此代码只是“复制粘贴”编程的结果。 但是，错误很可能会潜入这里。

V519'copiedPath [len]'变量连续两次被赋值。 也许这是一个错误。 检查行：92，93。kernel_emu.cpp 93

 int UserlandFS::KernelEmu::new_path(const char *path, char **copy) { .... // append a dot, if desired if (appendDot) { copiedPath[len] = '.'; copiedPath[len] = '\0'; } .... }

好的，程序员在复制方面很不幸。 符号“点”被添加到一行，并以终端空值重写。 作者很有可能只是复制了这一行而忘了增加索引。

奇怪的条件


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

 void FindPanel::BuildAttrQuery(BQuery* query, bool &dynamicDate) const { .... case B_BOOL_TYPE: { uint32 value; if (strcasecmp(textControl->Text(), "true") == 0) { value = 1; } else if (strcasecmp(textControl->Text(), "true") == 0) { value = 1; } else value = (uint32)atoi(textControl->Text()); value %= 2; query->PushUInt32(value); break; } .... }

复制代码一次导致两个错误。 条件表达式是相同的。 最有可能的是，其中之一必须与“ false”字符串而不是“ true”进行比较。 此外，在处理“假”值的分支中，应将值从1更改为0 。 该算法要求使用atoi函数将任何其他与true或false不同的值转换为数字。 但是由于错误，文本“ false”将进入函数。

V547表达式'错误==（（int）0）'始终为true。 Directory.cpp 688

 int32 BDirectory::CountEntries() { status_t error = Rewind(); if (error != B_OK) return error; int32 count = 0; BPrivate::Storage::LongDirEntry entry; while (error == B_OK) { if (GetNextDirents(&entry, sizeof(entry), 1) != 1) break; if (strcmp(entry.d_name, ".") != 0 && strcmp(entry.d_name, "..") != 0) count++; } Rewind(); return (error == B_OK ? count : error); }

分析器检测到错误变量值将始终为B_OK 。 最肯定的是，在while循环中错过了此变量修改。

V564 '＆'运算符应用于布尔类型值。 您可能忘记了括号或打算使用'&&'运算符。 strtod.c 545

 static int lo0bits(ULong *y) { int k; ULong x = *y; .... if (!(x & 1)) { k++; x >>= 1; if (!x & 1) // <= return 32; } *y = x; return k; }

与上面的条件一样，在最后一个条件表达式中最有可能忘记放置括号。 互补运算符可能不在括号内：

 if (!(x & 1)) // <= return 32;

V590考虑检查此表达式。 表达式过多或打印错误。 PoseView.cpp 5851

 bool BPoseView::AttributeChanged(const BMessage* message) { .... result = poseModel->OpenNode(); if (result == B_OK || result != B_BUSY) break; .... }

这不是很明显，但是条件的结果并不取决于B_OK值的值。 因此可以简化：

 If (result != B_BUSY) break;

您可以通过为结果变量的值绘制真值表来轻松地对其进行检查。 如果要专门考虑不同于B_OK和B_BUSY的其他值，则应以另一种方式重写代码。

另外两个类似的片段：

  • V590考虑检查此表达式。 表达式过多或打印错误。 Tracker.cpp 1714
  • V590考虑检查此表达式。 表达式过多或打印错误。 if_ipw.c 1871年

V590考虑检查'argc == 0 || argc！= 2'表达式。 表达式过多或打印错误。 cmds.c 2667

 void unsetoption(int argc, char *argv[]) { .... if (argc == 0 || argc != 2) { fprintf(ttyout, "usage: %s option\n", argv[0]); return; } .... }

这可能是演示V590诊断程序工作的最简单示例。 如果没有传递的参数，或者没有两个参数，则需要显示程序说明。 显然，除两个以外的任何值（包括零）都不会满足该条件。 因此，可以安全地将条件简化为：

 if (argc != 2) { fprintf(ttyout, "usage: %s option\n", argv[0]); return; }

V590考虑检查'* ptr ==';' && * ptr！='\ 0''表达式。 表达式过多或打印错误。 pc c 316

 ULONG parse_expression(char *str) { .... ptr = skipwhite(ptr); while (*ptr == SEMI_COLON && *ptr != '\0') { ptr++; if (*ptr == '\0') continue; val = assignment_expr(&ptr); } .... }

在此示例中，逻辑运算符已更改，但是逻辑仍然相同。 在这里，while循环的条件仅取决于字符是否等于SEMI_COLON 。

V590考虑检查此表达式。 表达式过多或打印错误。 writembr.cpp 99

 int main(int argc, char** argv) { .... string choice; getline(cin, choice, '\n'); if (choice == "no" || choice == "" || choice != "yes") { cerr << "MBR was NOT written" << endl; fs.close(); return B_ERROR; } .... }

在此示例中已经存在三个条件。 还可以在检查用户是否选择“是”之前进行简化：

 if (choice != "yes") { cerr << "MBR was NOT written" << endl; fs.close(); return B_ERROR; }

杂项


V530需要使用功能“开始”的返回值。 IMAPFolder.cpp 414

 void IMAPFolder::RegisterPendingBodies(...., const BMessenger* replyTo) { .... IMAP::MessageUIDList::const_iterator iterator = uids.begin(); for (; iterator != uids.end(); iterator++) { if (replyTo != NULL) fPendingBodies[*iterator].push_back(*replyTo); else fPendingBodies[*iterator].begin(); // <= } }

分析器发现迭代器begin（）的无意义调用。 我无法想象如何修复代码。 开发人员应注意此代码。

V609除以零。 分母范围[0..64]。 UiUtils.cpp 544

 static int32 GetSIMDFormatByteSize(uint32 format) { switch (format) { case SIMD_RENDER_FORMAT_INT8: return sizeof(char); case SIMD_RENDER_FORMAT_INT16: return sizeof(int16); case SIMD_RENDER_FORMAT_INT32: return sizeof(int32); case SIMD_RENDER_FORMAT_INT64: return sizeof(int64); case SIMD_RENDER_FORMAT_FLOAT: return sizeof(float); case SIMD_RENDER_FORMAT_DOUBLE: return sizeof(double); } return 0; } const BString& UiUtils::FormatSIMDValue(const BVariant& value, uint32 bitSize, uint32 format, BString& _output) { _output.SetTo("{"); char* data = (char*)value.ToPointer(); uint32 count = bitSize / (GetSIMDFormatByteSize(format) * 8); // <= .... }

函数GetSIMDFormatByteSize确实返回0作为默认值，这可能会导致被零除。

V654循环的条件“ specificSequence！= Sequence”始终为假。 pthread_key.cpp 55

 static void* get_key_value(pthread_thread* thread, uint32 key, int32 sequence) { pthread_key_data& keyData = thread->specific[key]; int32 specificSequence; void* value; do { specificSequence = keyData.sequence; if (specificSequence != sequence) return NULL; value = keyData.value; } while (specificSequence != sequence); keyData.value = NULL; return value; }

分析器是对的， 而while运算符的条件始终为假。 因此，该循环最多运行一次迭代。 换句话说，如果您在（0）期间写，则什么都不会改变。 这一切都很奇怪，并且此代码包含逻辑错误。 开发人员应仔细考虑此片段。

V672可能不需要在此处创建新的“ path”变量。 函数的参数之一具有相同的名称，并且该参数是引用。 检查行：348、429。translate.cpp 429

 status_t Translator::FindPath(...., TypeList &path, double &pathQuality) { .... TypeList path; double quality; if (FindPath(&formats[j], stream, typesSeen, path, quality) == B_OK) { if (bestQuality < quality * formatQuality) { bestQuality = quality * formatQuality; bestPath.SetTo(path); bestPath.Add(formats[j].type); status = B_OK; } } .... }

路径变量通过引用传递给FindPath函数。 这意味着可以在函数主体中修改此变量。 但是存在一个具有相同名称的局部变量，该变量已被修改。 在这种情况下，所有更改将仅保留在局部变量中。 代码作者可能想重命名或删除局部变量。

V705可能会忘记或注释掉“ else”块，从而改变了程序的操作逻辑。 主机名View.cpp 109

 status_t HostnameView::_LoadHostname() { BString fHostnameString; char hostname[MAXHOSTNAMELEN]; if (gethostname(hostname, MAXHOSTNAMELEN) == 0) { fHostnameString.SetTo(hostname, MAXHOSTNAMELEN); fHostname->SetText(fHostnameString); return B_OK; } else return B_ERROR; }

代码格式不良的示例。 “ hanging”关键字else不会改变逻辑，但是一旦在return运算符之前插入了代码片段，逻辑就不会相同。

V763参数“菜单”在使用前总是在功能体中重写。 video.cpp 648

 bool video_mode_hook(Menu *menu, MenuItem *item) { video_mode *mode = NULL; menu = item->Submenu(); item = menu->FindMarked(); .... }

我发现很多情况下，进入函数时重写函数参数。 这种行为会误导其他调用这些功能的开发人员。

整个可疑地点清单：

  • V763参数'force_16bit'始终在使用前在功能体内重写。 ata_adapter.cpp 151
  • V763参数'force_16bit'始终在使用前在功能体内重写。 ata_adapter.cpp 179
  • V763参数“菜单”在使用前总是在功能体中重写。 video.cpp 264
  • V763参数“长度”在使用前总是在功能体中重写。 MailMessage.cpp 677
  • V763参数“入口”在使用前总是在功能体中重写。 IconCache.cpp 773
  • V763参数“入口”在使用前总是在功能体中重写。 IconCache.cpp 832
  • V763参数“入口”在使用前总是在功能体中重写。 IconCache.cpp 864
  • V763参数“ rect”在使用前总是在功能体中重写。 ErrorLogWindow.cpp 56
  • V763参数'updateRect'始终在使用前在函数体中重写。 CalendarMenuWindow.cpp 49
  • V763参数“ rect”在使用前总是在功能体中重写。 MemoryView.cpp 165
  • V763参数“ rect”在使用前总是在功能体中重写。 TypeEditors.cpp 1124
  • V763参数“高度”在使用前总是在功能体内重写。 工作空间.cpp 857
  • V763参数“宽度”在使用前总是在功能体内重写。 工作区.cpp 856
  • V763参数“框架”在使用前总是在功能体中重写。 SwatchGroup.cpp 48
  • V763参数“框架”在使用前总是在功能体中重写。 播放列表窗口.cpp 89
  • V763参数“ rect”在使用前总是在功能体中重写。 ConfigView.cpp 78
  • V763参数“ m”在使用前总是在功能体内被重写。 mkntfs.c 3917
  • V763参数“ rxchainmask”在使用前总是在功能体中重写。 ar5416_cal.c 463
  • V763参数“ c”在使用前总是在功能体内被重写。 if_iwn.c 6854

结论


Haiku项目是有趣而罕见的错误的来源。 我们已经在数据库中添加了一些错误示例，并修复了一些在分析代码时出现的分析器问题。

如果您很长一段时间没有使用某些代码分析工具检查代码，那么我描述的一些问题可能隐藏在您的代码中。 在您的项目（如果使用C，C ++，C＃或Java编写）中使用PVS-Studio来控制代码质量。 无需注册或发送短信即可在此处下载分析仪。

您想尝试Haiku吗？ Haiku开发人员邀请您加入电报频道 。

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

More articles:


All Articles