Geekly articles weekly

NCBI基因组工作台：濒危研究

现代计算机技术，技术和软件解决方案-所有这些都极大地促进和加速了各种科学研究的实施。 通常，计算机仿真是检验许多理论的唯一方法。 科学软件具有自己的特点。 例如，此类软件通常会经过非常全面的测试，但记录不充分。 但是，软件是人们编写的，人们会犯错误。 科学程序中的错误可能会对整个研究产生怀疑。 本文将列出在NCBI Genome Workbench软件包的代码中发现的许多问题。

引言


NCBI基因组工作台为研究人员提供了广泛的工具，用于研究和分析遗传数据。 用户可以研究和比较来自多个来源的数据，包括NCBI（国家生物技术信息中心）数据库或他们自己的个人数据。

如前所述，单元测试通常涵盖科学软件。 检查该项目时，分析中排除了包含测试文件的85个目录。 这大约是一千个文件。 也许这是由于需要测试用于各种研究而发明的各种复杂算法。 但是其余代码（不是测试代码）的质量却没有我们想要的那么高。 但是，就像在尚未考虑引入静态代码分析工具的任何项目中一样：)。

静态代码分析器为C / C ++ / C＃/ Java- PVS-Studio提供了用于代码审查（或研究）的数据。

只有两个数字会破坏您的项目




基于我们的错误数据库，当前有超过一万两千个示例被选择，我们注意到并描述了编写导致大量错误的代码的特定模式。 例如，我们进行了以下研究：

  1. 最后一行的效果 ;
  2. C / C ++世界上最危险的函数 ;
  3. C / C ++中的逻辑表达式。 专业人士有多错误 ;
  4. 邪恶生活在比较功能中 。

该项目标志着新模式描述的开始。 我们正在谈论变量名称中的数字1和2 ，例如file1和file2等。 容易混淆两个这样的变量。 这是代码中拼写错误的一种特殊情况，但是一个这样的错误导致人们希望使用同名变量，它们之间的区别仅在于名称末尾的数字1和2。

展望未来，我会说上面列出的所有研究都已在该项目的代码D中得到确认。

考虑Genome Workbench项目的第一个示例：

V501在“ ||”的左侧和右侧有相同的子表达式“（！Loc1.IsInt（）&&！Loc1.IsWhole（））” 操作员。 nw_aligner.cpp 480

CRef<CSeq_align> CNWAligner::Run(CScope &scope, const CSeq_loc &loc1, const CSeq_loc &loc2, bool trim_end_gaps) { if ((!loc1.IsInt() && !loc1.IsWhole()) || (!loc1.IsInt() && !loc1.IsWhole())) { NCBI_THROW(CException, eUnknown, "Only whole and interval locations supported"); } .... }

我们看到两个名为loc1和loc2的变量。 代码中还有一个错误：未使用loc2变量，因为代替了它，再次使用了loc1 。

另一个例子：

V560条件表达式的一部分始终为false：s1.IsSet（）。 valid_biosource.cpp 3073

 static bool s_PCRPrimerSetLess(const CPCRPrimerSet& s1, const CPCRPrimerSet& s2) { if (!s1.IsSet() && s1.IsSet()) { return true; } else if (s1.IsSet() && !s2.IsSet()) { return false; } else if (!s1.IsSet() && !s2.IsSet()) { return false; } else if (s1.Get().size() < s2.Get().size()) { return true; } else if (s1.Get().size() > s2.Get().size()) { return false; } else { ..... }

代码的第一行将变量s1和s2混合在一起。 根据名称，这是一个比较功能。 但是这种错误可能随处可见，因为通过命名变量Number 1和Number 2 ，程序员几乎肯定会在将来犯一个错误。 并且在函数中使用此类名称的次数越多，出错的可能性就越高。

其他错别字和复制粘贴




V501 '！='运算符左右两侧有相同的子表达式：bd.bit_.bits [i]！= Bd.bit_.bits [i] bm.h 296

 bool compare_state(const iterator_base& ib) const { .... if (this->block_type_ == 0 { if (bd.bit_.ptr != ib_db.bit_.ptr) return false; if (bd.bit_.idx != ib_db.bit_.idx) return false; if (bd.bit_.cnt != ib_db.bit_.cnt) return false; if (bd.bit_.pos != ib_db.bit_.pos) return false; for (unsigned i = 0; i < bd.bit_.cnt; ++i) { if (bd.bit_.bits[i] != bd.bit_.bits[i]) return false; } } .... }

我相信经过所有检查之后， bd.bit_和ib_db.bit_对象的位数组的大小是相等的。 因此，该代码的作者编写了一个循环来对位数组进行元素比较，但以其中一个比较对象的名称打错了字 。 结果，在某些情况下，被比较的对象可能被错误地认为是相等的。

这个例子值得“ 邪恶生活在比较功能中 ”一文。

V501在“ ||”的左侧和右侧有相同的子表达式“ CFieldHandler :: QualifierNamesAreEquivalent（字段，kFieldTypeSeqId）” 操作员。 field_handler.cpp 152

 bool CFieldHandlerFactory::s_IsSequenceIDField(const string& field) { if ( CFieldHandler::QualifierNamesAreEquivalent(field, kFieldTypeSeqId) || CFieldHandler::QualifierNamesAreEquivalent(field, kFieldTypeSeqId)) { return true; } else { return false; } }

最有可能的检查是多余的。 我没有在代码变量中找到类似于kFieldTypeSeqId的变量。 但是，由于使用“ ||”运算符，可能会导致额外的函数调用，从而降低性能。

另外两个带有分析仪警告的相同类型的场所，需要进行验证：

  • V501在'&&'运算符的左侧和右侧有相同的子表达式'uf-> GetData（）。IsBool（）'。 variant_utils.cpp 1711
  • V501在'&&'运算符的左侧和右侧有相同的子表达式'uf-> GetData（）。IsBool（）'。 variant_utils.cpp 1735

V766已经添加了具有相同键“ kArgRemote”的项目。 blast_args.cpp 3262

 void CBlastAppArgs::x_IssueWarningsForIgnoredOptions(const CArgs& args) { set<string> can_override; .... can_override.insert(kArgOutputFormat); can_override.insert(kArgNumDescriptions); can_override.insert(kArgNumAlignments); can_override.insert(kArgMaxTargetSequences); can_override.insert(kArgRemote); // <= can_override.insert(kArgNumThreads); can_override.insert(kArgInputSearchStrategy); can_override.insert(kArgRemote); // <= can_override.insert("remote_verbose"); can_override.insert("verbose"); .... }

分析仪检测到向设置的容器添加了两个相同的值。 回想一下，该容器仅存储唯一值，因此不会向其添加重复项。

类似于上述代码的代码通常是使用copy-paste方法编写的。 可能仅仅是一个额外的值，或者也许作者在复制时忘记重命名其中一个变量。 当您删除一个额外的插入调用时，代码会稍作优化，但这并不重要。 更重要的是，由于集合中缺少元素，可能会在这里隐藏严重错误。

V523'then '语句等效于后续代码片段。 vcf_reader.cpp 1105

 bool CVcfReader::xAssignFeatureLocationSet(....) { .... if (data.m_SetType == CVcfData::ST_ALL_DEL) { if (data.m_strRef.size() == 1) { //deletion of a single base pFeat->SetLocation().SetPnt().SetPoint(data.m_iPos-1); pFeat->SetLocation().SetPnt().SetId(*pId); } else { pFeat->SetLocation().SetInt().SetFrom(data.m_iPos-1); //-1 for 0-based, //another -1 for inclusive end-point ( ie [], not [) ) pFeat->SetLocation().SetInt().SetTo( data.m_iPos -1 + data.m_strRef.length() - 1); pFeat->SetLocation().SetInt().SetId(*pId); } return true; } //default: For MNV's we will use the single starting point //NB: For references of size >=2, this location will not //match the reference allele. Future Variation-ref //normalization code will address these issues, //and obviate the need for this code altogether. if (data.m_strRef.size() == 1) { //deletion of a single base pFeat->SetLocation().SetPnt().SetPoint(data.m_iPos-1); pFeat->SetLocation().SetPnt().SetId(*pId); } else { pFeat->SetLocation().SetInt().SetFrom(data.m_iPos-1); pFeat->SetLocation().SetInt().SetTo( data.m_iPos -1 + data.m_strRef.length() - 1); pFeat->SetLocation().SetInt().SetId(*pId); } return true; }

该函数包含大型且完全相同的代码片段。 但是，它们包含各种附带的注释。 该代码的编写不是最佳，令人困惑的，并且可能包含错误。

带有if-else语句的可疑场所的完整列表如下所示：

  • V523'then'语句等效于'else'语句。 第2142章
  • V523'then'语句等效于后续代码片段。 odbc.c 379
  • V523'then'语句等效于后续代码片段。 odbc.c 1414
  • V523'then'语句等效于'else'语句。 seqdbvol.cpp 1922年
  • V523'then'语句等效于'else'语句。 seqdb_demo.cpp 466
  • V523'then'语句等效于后续代码片段。 blast_engine.c 1917年
  • V523'then'语句等效于'else'语句。 blast_filter.c 420
  • V523'then'语句等效于'else'语句。 blast_parameters.c 636
  • V523'then'语句等效于'else'语句。 第684章
  • V523'then'语句等效于'else'语句。 bme.cpp 333
  • V523'then'语句等效于'else'语句。 gme.cpp 484

/ *具有安全性的最好是书呆子* /



V597编译器可以删除“ 内存集 ”函数调用，该函数调用用于刷新“ passwd_buf”缓冲区。 memset_s（）函数应用于擦除私有数据。 第366章

 /** * Crypt a given password using schema required for NTLMv1 authentication * @param passwd clear text domain password * @param challenge challenge data given by server * @param flags NTLM flags from server side * @param answer buffer where to store crypted password */ void tds_answer_challenge(....) { #define MAX_PW_SZ 14 .... if (ntlm_v == 1) { .... /* with security is best be pedantic */ memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); memset(ntlm2_challenge, 0, sizeof(ntlm2_challenge)); } else { .... } }

您可能已经猜到了，在节标题中使用了关于代码安全性的有趣注释。

简而言之，由于不再使用刷新缓冲区，因此编译器将删除memset函数。 而且像hash或passwd_buf这样的数据实际上不会为零。 有关此非显而易见的编译器机制的更多信息，请参见文章“ 安全清除私有数据 ”。

V597编译器可以删除“ 内存集 ”函数调用，该函数调用用于刷新“答案”对象。 memset_s（）函数应用于擦除私有数据。 第561章

 static TDSRET tds7_send_auth(....) { .... /* for security reason clear structure */ memset(&answer, 0, sizeof(TDSANSWER)); return tds_flush_packet(tds); }

那不是唯一评论“安全”的例子。 从评论来看，可以假设安全性对于该项目确实非常重要。 因此，我附上已发现问题的整个非小清单：

  • V597编译器可能会删除“ memset”函数调用，该函数调用用于刷新“堆”对象。 memset_s（）函数应用于擦除私有数据。 ncbi_heapmgr.c 1300
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“上下文”对象。 memset_s（）函数应用于擦除私有数据。 第167章
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ ks”对象。 memset_s（）函数应用于擦除私有数据。 挑战c 339
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ md5_ctx”对象。 memset_s（）函数应用于擦除私有数据。 第353章
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“哈希”缓冲区。 memset_s（）函数应用于擦除私有数据。 Challenge.c 365
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ ks”对象。 memset_s（）函数应用于擦除私有数据。 挑战c 406
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ ntlm_v2_response”对象。 memset_s（）函数应用于擦除私有数据。 login.c 795
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“答案”对象。 memset_s（）函数应用于擦除私有数据。 login.c 801
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“数据包”缓冲区。 memset_s（）函数应用于擦除私有数据。 数值c 256
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“数据包”缓冲区。 memset_s（）函数应用于擦除私有数据。 数字c 110
  • V597编译器可能会删除“ memset”函数调用，该函数调用用于刷新“ pwd”缓冲区。 memset_s（）函数应用于擦除私有数据。 getpassarg.c 50
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“上下文”对象。 memset_s（）函数应用于擦除私有数据。 挑战c 188
  • V597编译器可能会删除“ memset”函数调用，该函数调用用于刷新“ buf”缓冲区。 memset_s（）函数应用于擦除私有数据。 第243章
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ ntlm_v2_hash”缓冲区。 memset_s（）函数应用于擦除私有数据。 挑战c 309
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ md5_ctx”对象。 memset_s（）函数应用于擦除私有数据。 挑战c 354
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ passwd_buf”缓冲区。 memset_s（）函数应用于擦除私有数据。 挑战c 380
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ ks”对象。 memset_s（）函数应用于擦除私有数据。 第393章
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“哈希”缓冲区。 memset_s（）函数应用于擦除私有数据。 挑战c 394
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ ntlm2_challenge”缓冲区。 memset_s（）函数应用于擦除私有数据。 挑战c 395
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ ks”对象。 memset_s（）函数应用于擦除私有数据。 挑战c 419
  • V597编译器可以删除“内存集”函数调用，该函数调用用于刷新“ ntlm_v2_response”对象。 memset_s（）函数应用于擦除私有数据。 第556章

可疑周期




V534可能在“ for”运算符内比较了错误的变量。 考虑查看“ i”。 taxFormat.cpp 569

 void CTaxFormat::x_LoadTaxTree(void) { .... for(size_t i = 0; i < alignTaxids.size(); i++) { int tax_id = alignTaxids[i]; .... for(size_t j = 0; i < taxInfo.seqInfoList.size(); j++) { SSeqInfo* seqInfo = taxInfo.seqInfoList[j]; seqInfo->taxid = newTaxid; } .... } .... }

我认为，在内部循环的条件下，变量是我随机获得的。 相反，应该使用变量j 。

V535变量“ i”用于该循环和外部循环。 检查行：302、309。sls_alp.cpp 309

 alp::~alp() { .... if(d_alp_states) { for(i=0;i<=d_nalp;i++) // <= { if(i<=d_alp_states->d_dim) { if(d_alp_states->d_elem[i]) { for(i=0;i<=d_nalp;i++) // <= { .... .... }

两个嵌套的相同循环（其中还重置了全局计数器）看起来非常可疑。 开发人员应检查此处发生的情况。

数组索引异常




V520数组索引表达式'[-i2，-k]'中的逗号运算符'，'。 nw_spliced_aligner16.cpp 564

 void CSplicedAligner16::x_DoBackTrace ( const Uint2* backtrace_matrix, CNWAligner::SAlignInOut* data, int i_global_max, int j_global_max) { .... while(intron_length < m_IntronMinSize || (Key & donor) == 0) { Key = backtrace_matrix[--i2, --k]; ++intron_length; data->m_transcript.push_back(eTS_Intron); } .... }

我必须马上说，似乎没有错误（目前，大声笑）。 考虑以下行：

 Key = backtrace_matrix[--i2, --k];

单词“矩阵”和双索引可能暗示该数组是二维的，但不是二维的。 这是指向整数数组的常规指针。 但是V520诊断程序并没有出现。 程序员对于如何索引二维数组确实感到困惑。

在这种情况下，作者只是决定保存一行代码，尽管他可以这样写：

 --i2; Key = backtrace_matrix[--k];

V661可疑表达式'A [B == C]'。 可能是“ A [B] == C”。 ncbi_service_connector.c 180

 static EHTTP_HeaderParse s_ParseHeader(const char* header, ....) { .... if (sscanf(header, "%u.%u.%u.%u%n", &i1, &i2, &i3, &i4, &n) < 4 || sscanf(header + n, "%hu%x%n", &uuu->port, &tkt, &m) < 2 || (header[m += n] && !(header[m] == '$') && !isspace((unsigned char)((header + m) [header[m] == '$'])))) { break/*failed - unreadable connection info*/; } .... }

我花了很长时间试图了解正在发生的事情的另一个代码示例：D. isspace（）函数检查索引为m的字符，但是如果此字符为'$'，则索引为m +1的字符将传递给该函数。 此外，与“ $”的比较已经提前。 也许这里没有错误，但是绝对可以更清楚地重写代码。

V557阵列可能超限。 “行”索引指向数组边界之外。 aln_reader.cpp 412

 bool CAlnReader::x_IsGap(TNumrow row, TSeqPos pos, const string& residue) { if (m_MiddleSections.size() == 0) { x_CalculateMiddleSections(); } if (row > m_MiddleSections.size()) { return false; } if (pos < m_MiddleSections[row].first) { .... } .... }

这里有一个严重的错误。 正确的行索引检查应如下所示：

 if (row >= m_MiddleSections.size()) { return false; }

否则，可以访问MiddleSections向量外部的数据。

还有更多这样的地方：

  • V557阵列可能超限。 “ i”索引指向数组界限之外。 resource_pool.hpp 388
  • V557阵列可能超限。 “行”索引指向数组边界之外。 aln_reader.cpp 418
  • V557阵列可能超限。 “ fmt_idx”索引指向数组界限之外。 seq_writer.cpp 384
  • V557阵列可能超限。 “ fmt_idx”索引指向数组界限之外。 blastdb_formatter.cpp 183
  • V557阵列可能超限。 'num'索引指向数组边界之外。 newcleanupp.cpp 13035

如何获得对功能的不信任




V570'm_onClickFunction'变量被分配给它自己。 alngraphic.hpp 103

 void SetOnClickFunctionName(string onClickFunction) { m_onClickFunction = m_onClickFunction; }

甚至没有什么可评论的。 您只能同情单击某物，单击某物但没有任何变化的人。

将变量分配给自己的另外两种情况将产生一个列表：

  • V570将'iter-> level'变量分配给它自己。 align_format_util.cpp 189
  • V570将'd_elements_values [ind]'变量分配给它自己。 sls_alp_data.cpp 1416

V763参数“ w1”在使用前总是在功能体中重写。 bmfunc.h 5363

 /// Bit COUNT functor template<typename W> struct bit_COUNT { W operator()(W w1, W w2) { w1 = 0; BM_INCWORD_BITCOUNT(w1, w2); return w1; } };

在进入函数后立即磨损参数的函数可能会误导使用它的开发人员。 该代码应仔细检查。

类设计错误




V688'm_qsrc '函数参数具有与类成员之一相同的名称，这可能导致混淆。 第873章

 class CElementaryMatching: public CObject { .... ISequenceSource * m_qsrc; .... void x_CreateIndex (ISequenceSource *m_qsrc, EIndexMode index_more, ....); void x_CreateRemapData(ISequenceSource *m_qsrc, EIndexMode mode); void x_LoadRemapData (ISequenceSource *m_qsrc, const string& sdb); .... };

立即有3个类函数包含名称与类字段一致的参数。 这可能导致函数体出错：程序员可能会认为他正在与一个类成员一起工作，实际上是在更改局部变量的值。

V614使用了未初始化的变量'm_BitSet'。 SnpBitAttributes.hpp 187

 /// SNP bit attribute container. class CSnpBitAttributes { public: .... private: /// Internal storage for bits. Uint8 m_BitSet; }; inline CSnpBitAttributes::CSnpBitAttributes(Uint8 bits) : m_BitSet(bits) { } inline CSnpBitAttributes::CSnpBitAttributes(const vector<char>& octet_string) { auto count = sizeof(m_BitSet); auto byte = octet_string.end(); do m_BitSet = (m_BitSet << 8) | *--byte; while (--count > 0); }

构造函数之一可随意使用m_BitSet变量。 事实是该变量未初始化。 其“垃圾”值在循环的第一次迭代中使用，此后发生初始化。 这是一个非常严重的错误，导致未定义的程序行为。

V603对象已创建，但未被使用。 如果要调用构造函数，则应使用“ this-> SIntervalComparisonResult :: SIntervalComparisonResult（....）”。 compare_feats.hpp 100

 //This struct keeps the result of comparison of two exons struct SIntervalComparisonResult : CObject { public: SIntervalComparisonResult(unsigned pos1, unsigned pos2, FCompareLocs result, int pos_comparison = 0) : m_exon_ordinal1(pos1), m_exon_ordinal2(pos2), m_result(result), m_position_comparison(pos_comparison) {} SIntervalComparisonResult() { SIntervalComparisonResult(0, 0, fCmp_Unknown, 0); } .... };

很久以前，在检查项目时我没有遇到这样的错误。 但是问题仍然存在。 错误是，以这种方式调用参数化构造函数会创建并删除一个临时对象。 并且类字段保持未初始化。 应该通过初始化列表调用另一个构造函数（请参阅Delegating构造函数 ）。

V591非无效函数应返回一个值。 第266章

 /// Recursive assignment CBioNode& operator=(const CBioNode& tree) { TParent::operator=(tree); TBioTree* pt = (TBioTree*)tree.GetParentTree(); SetParentTree(pt); }

分析器认为该行在重载语句中丢失：

 return *this;

V670未初始化的类成员'm_OutBlobIdOrData'用于初始化'm_StdOut'成员。 请记住，成员是按照类内声明的顺序进行初始化的。 remote_app.hpp 215

 class NCBI_XCONNECT_EXPORT CRemoteAppResult { public: CRemoteAppResult(CNetCacheAPI::TInstance netcache_api, size_t max_inline_size = kMaxBlobInlineSize) : m_NetCacheAPI(netcache_api), m_RetCode(-1), m_StdOut(netcache_api, m_OutBlobIdOrData, m_OutBlobSize), m_OutBlobSize(0), m_StdErr(netcache_api, m_ErrBlobIdOrData, m_ErrBlobSize), m_ErrBlobSize(0), m_StorageType(eBlobStorage), m_MaxInlineSize(max_inline_size) { } .... };

系统会立即对此代码段发出3个分析器警告。 类字段的初始化不是按照它们在初始化列表中列出的顺序进行，而是按照它们在类中声明的方式进行。 导致此错误的经典原因是，并非所有程序员都记得或知道此规则。 在这里和初始化列表中的顺序是错误的。 人们会感觉到字段列表是以随机顺序输入的。

V746对象切片。 应该通过引用而不是值来捕获异常。 钴cpp 247

 void CMultiAligner::SetQueries(const vector< CRef<objects::CBioseq> >& queries) { .... try { seq_loc->SetId(*it->GetSeqId()); } catch (objects::CObjMgrException e) { NCBI_THROW(CMultiAlignerException, eInvalidInput, (string)"Missing seq-id in bioseq. " + e.GetMsg()); } m_tQueries.push_back(seq_loc); .... }

由于创建新对象，按值捕获异常可能导致丢失有关该异常的某些信息。 通过引用捕获异常会更好，更安全。

类似的地方：

  • V746对象切片。 应该通过引用而不是值来捕获异常。 agp_validate_reader.cpp 562
  • V746对象切片。 应该通过引用而不是值来捕获异常。 aln_build_app.cpp 320
  • V746对象切片。 应该通过引用而不是值来捕获异常。 458第458章
  • V746对象切片。 应该通过引用而不是值来捕获异常。 钴cpp 691
  • V746对象切片。 应该通过引用而不是值来捕获异常。 钴cpp 719
  • V746对象切片。 应该通过引用而不是值来捕获异常。 钴cpp 728
  • V746对象切片。 应该通过引用而不是值来捕获异常。 钴cpp 732

关于无法访问的代码和其他代码执行问题



V779检测到无法访问的代码。 可能存在错误。 merge_tree_core.cpp 627

 bool CMergeTree::x_FindBefores_Up_Iter(....) { .... FirstFrame->Curr = StartCurr; FirstFrame->Returned = false; FirstFrame->VisitCount = 0; FrameStack.push_back(FirstFrame); while(!FrameStack.empty()) { .... if(Rel == CEquivRange::eAfter) { Frame->Returned = false; FrameStack.pop_back(); continue; } else if(Rel == CEquivRange::eBefore) { .... continue; } else { if(Frame->VisitCount == 0) { .... continue; } else { .... continue; } } Frame->Returned = false; // <= FrameStack.pop_back(); continue; } // end stack loop FirstFrame->ChildFrames.clear(); return FirstFrame->Returned; }

编写条件语句的代码，以便代码的绝对所有分支都以continue语句结尾。 这导致在while循环中形成几行无法访问的代码。 这些行看起来非常可疑。 最有可能的是，此问题在重构代码后出现，现在需要仔细的代码审查。

V519连续两次给'interval_width'变量赋值两次。 也许这是一个错误。 检查行：454、456。aln_writer.cpp 456

 void CAlnWriter::AddGaps(....) { .... switch(exon_chunk->Which()) { case CSpliced_exon_chunk::e_Match: interval_width = exon_chunk->GetMatch(); case CSpliced_exon_chunk::e_Mismatch: interval_width = exon_chunk->GetMismatch(); case CSpliced_exon_chunk::e_Diag: interval_width = exon_chunk->GetDiag(); genomic_string.append(....); product_string.append(....); genomic_pos += interval_width; product_pos += interval_width/res_width; break; .... } .... }

变量interval_width被多次覆盖，因为 在case分支中没有break语句。 虽然是经典的，但是却是一个非常糟糕的错误。

其他一些可疑的地方：

  • V779检测到无法访问的代码。 可能存在错误。 dbapi_driver_utils.cpp 351
  • V779检测到无法访问的代码。 可能存在错误。 网络780
  • V779检测到无法访问的代码。 可能存在错误。 bcp.c 1495
  • V779检测到无法访问的代码。 可能存在错误。 remote_blast.cpp 1470
  • V779检测到无法访问的代码。 可能存在错误。 remote_blast.cpp 1522

V571定期检查。 “ if（m_QueryOpts-> filtering_options）”条件已经在第703行中进行了验证。blast_options_local_priv.hpp 713

 inline void CBlastOptionsLocal::SetFilterString(const char* f) { .... if (m_QueryOpts->filtering_options) // <= { SBlastFilterOptions* old_opts = m_QueryOpts->filtering_options; m_QueryOpts->filtering_options = NULL; SBlastFilterOptionsMerge(&(m_QueryOpts->filtering_options), old_opts, new_opts); old_opts = SBlastFilterOptionsFree(old_opts); new_opts = SBlastFilterOptionsFree(new_opts); } else { if (m_QueryOpts->filtering_options) // <= m_QueryOpts->filtering_options = SBlastFilterOptionsFree(m_QueryOpts->filtering_options); m_QueryOpts->filtering_options = new_opts; new_opts = NULL; } .... }

显然， else分支需要重写。 我有一些想使用m_QueryOpts-> filtering_options指针的想法 ，但是代码仍然在某种程度上令人困惑。 我呼吁代码的作者。

好吧，这个问题并不孤单：

  • V571定期检查。 “ if（睡眠时间）”条件已在第205行中得到验证。request_control.cpp 208
  • V571定期检查。 “ if（assignValue.empty（））”条件已在第712行中得到验证。classstr.cpp 718

数据读取错误



V739 EOF不应与“ char”类型的值进行比较。 “ linestring [0]”应为“ int”类型。 第3509章

 static EBool s_AfrpInitLineData( .... char* linestring = readfunc (pfile); .... while (linestring != NULL && linestring [0] != EOF) { s_TrimSpace (&linestring); .... } .... }

计划与EOF比较的字符不应存储在char变量中。 否则，存在值0xFF（255）的字符将变为-1并以与文件末尾（EOF）相同的方式进行解释的风险。 同样（以防万一）值得检查readfunc函数的实现。

V663可能出现无限循环。 'cin.eof（）'条件不足以使它脱离循环。考虑将'cin.fail（）'函数调用添加到条件表达式中。ncbicgi.cpp 1564

 typedef std::istream CNcbiIstream; void CCgiRequest::Serialize(CNcbiOstream& os) const { .... CNcbiIstream* istrm = GetInputStream(); if (istrm) { char buf[1024]; while(!istrm->eof()) { istrm->read(buf, sizeof(buf)); os.write(buf, istrm->gcount()); } } }

分析仪检测到潜在错误，由于该错误可能发生无限循环。如果在读取数据时发生故障，则调用eof（）函数将始终返回false。为了在这种情况下完成循环，必须对fail（）函数返回的值进行额外检查。

杂项错误



V502也许'？：'运算符的工作方式与预期的不同。'？：'运算符的优先级低于'&&'运算符。ncbi_connutil.c 1135

 static const char* x_ClientAddress(const char* client_host, int/*bool*/ local_host) { .... if ((client_host == c && x_IsSufficientAddress(client_host)) || !(ip = *c && !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(eDefault)) || SOCK_ntoa(ip, addr, sizeof(addr)) != 0 || !(s = (char*) malloc(strlen(client_host) + strlen(addr) + 3))) { return client_host/*least we can do :-/*/; } .... }

注意表达式：

 !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(eDefault)


它不是按程序员期望的那样计算的，因为整个表达式如下所示：

 ip = *c && !local_host ? SOCK_gethostbyname(c) : SOCK_GetLocalHostAddress(...)


&&运算符的优先级高于？：。因此，代码无法按预期执行。

V561给“ seq”变量赋值可能比重新声明它更好。先前的声明：validator.cpp，第490行。validator.cpp492

 bool CValidator::IsSeqLocCorrectlyOrdered(const CSeq_loc& loc, CScope& scope) { CBioseq_Handle seq; try { CBioseq_Handle seq = scope.GetBioseqHandle(loc); } catch (CObjMgrException& ) { // no way to tell return true; } catch (const exception& ) { // no way to tell return true; } if (seq && seq.GetInst_Topology() == CSeq_inst::eTopology_circular) { // no way to check if topology is circular return true; } return CheckConsecutiveIntervals(loc, scope, x_IsCorrectlyOrdered); }

由于程序员在try / catch部分中声明了一个新的seq变量，因此另一个seq变量仍未初始化，在下面的代码中使用。

V562将布尔类型值与0进行比较是奇怪的：（（（status）＆0x7f）== 0）！= 0. ncbi_process.cpp 111

 bool CProcess::CExitInfo::IsExited(void) const { EXIT_INFO_CHECK; if (state != eExitInfo_Terminated) { return false; } #if defined(NCBI_OS_UNIX) return WIFEXITED(status) != 0; #elif defined(NCBI_OS_MSWIN) // The process always terminates with exit code return true; #endif }

一切都没有预兆，但事实证明WIFEXITED是一个宏大的开头：

 return (((status) & 0x7f) == 0) != 0;

事实证明，该函数返回相反的值。

在代码中，还有另一个这样的函数，它发出警告：

  • V562将布尔类型值与0进行比较是奇怪的。ncbi_process.cpp 126

V595在针对nullptr对其进行验证之前，已使用了'dst_len'指针。检查行：309、315。zlib.cpp 309

 bool CZipCompression::CompressBuffer( const void* src_buf, size_t src_len, void* dst_buf, size_t dst_size, /* out */ size_t* dst_len) { *dst_len = 0; // Check parameters if (!src_len && !F_ISSET(fAllowEmptyData)) { src_buf = NULL; } if (!src_buf || !dst_buf || !dst_len) { SetError(Z_STREAM_ERROR, "bad argument"); ERR_COMPRESS(48, FormatErrorMessage("CZipCompression::CompressBuffer")); return false; } .... }

dst_len指针在函数的最开始处被取消引用，同时进一步检查代码是否等于零。如果dst_len指针为nullptr，则代码中发生错误，导致未定义的行为。

V590考虑检查'ch！='\ 0'&& ch =='''表达式。表达式过多或打印错误。cleanup_utils.cpp 580

 bool Asn2gnbkCompressSpaces(string& val) { .... while (ch != '\0' && ch == ' ') { ptr++; ch = *ptr; } .... }

停止循环的条件仅取决于字符ch是否为空格。该表达式可以简化为以下形式：

 while (ch == ' ') { .... }

结论


在科学研究中使用计算机程序可以帮助并且将有助于发现。我们希望不要因为错别字而错过特别重要的内容。

我邀请NCBI Genome Workbench项目的开发人员与我们联系，我们将提供由PVS-Studio分析仪发布的完整报告。

希望这个小小的代码研究可以帮助修复许多错误，并总体上提高项目的可靠性。如果尚未在项目代码上运行PVS-Studio，请尝试这样做。您可能会喜欢它:)。



如果您想与说英语的读者分享这篇文章，请使用以下链接：Svyatoslav Razmyslov。NCBI基因组工作台：受威胁的科学研究

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

More articles:


All Articles