Geekly articles weekly

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

大量错别字和复制粘贴代码成为PVS-Studio分析仪关于Haiku代码验证的另一篇文章的主题。 但是，错误不仅与错别字有关，还与粗心大意和重构失败有关。 发现的错误示例说明了人为因素在软件开发中的强大程度。

图片1

引言


Haiku是用于个人计算机的免费开放源代码操作系统。 当前，一个国际开发人员小组正在积极研究系统的组件。 在最近的重大开发中，有将LibreOffice移植到操作系统上以及R1 Beta 1的第一个beta版本的发布。

自2015年以来， PVS-Studio静态代码分析器的开发人员团队监视该项目的发展，发布代码缺陷评论。 这是有史以来的第四次评论。 您可以通过以下链接熟悉以前的文章：

  1. 检查Haiku操作系统（BeOS系列）。 第一部分
  2. 检查Haiku操作系统（BeOS系列）。 第二部分 ;
  3. 如何在C和C ++中放纵自己。 Haiku OS食谱集

最新代码分析的功能是可以使用Linux的正式版PVS-Studio。 在2015年，它不存在了，还有查看错误的便捷报告。 现在，我们将以方便的格式向开发人员发送完整的报告。

流派的经典


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指针初始化为零，然后在进入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; } .... }

我相信在将目录指针与空值进行比较时出现错误，条件应该相反。 在当前实现中，如果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; }

输入指针被初始化为零并保持在该值，因为 在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指针时进行了输入错误。 最有可能的是，函数的退出位置应为该指针的零值：

 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数组仅包含两个元素，而在代码中，它由常量BT_SCO进行访问，该值的值为3。保证发生了超出数组边界的退出。

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数组的调用（超出了数组的范围）。 原因是错误的代码顺序。 在这里，首先访问数组，然后检查索引值是否有效。

有人会说不会有麻烦。 毕竟，这里没有检索到数组元素的值，但是仅获取了单元地址。 不，您无论如何都无法做到。 阅读更多：“ 取消引用空指针会导致未定义的行为 。”

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”。 接下来，在处理值“ false”的分支中，应将值从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语句的条件始终为false是正确的。 因此，一个循环最多执行一次迭代。 换句话说，如果您在（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; }

不良代码设计的一个例子。 “ frozen” 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来控制代码的质量。 您可以在此处下载分析仪而无需注册和SMS。

想尝试Haiku并有任何疑问吗？ Haiku开发人员邀请您访问电报频道 。



如果您想与讲英语的读者分享这篇文章，请使用以下链接：Svyatoslav Razmyslov。 适用于C和C ++的最佳复制粘贴算法。 Haiku OS食谱

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

More articles:


All Articles