Geekly articles weekly

NCBI Genome Workbench: Pesquisa em Perigo

Tecnologias modernas de computador, soluções técnicas e de software - tudo isso facilita e acelera a implementação de várias pesquisas científicas. Freqüentemente, a simulação por computador é a única maneira de testar muitas teorias. O software científico tem suas próprias características. Por exemplo, esse software geralmente é submetido a testes muito completos, mas é pouco documentado. No entanto, o software é escrito por pessoas, e as pessoas cometem erros. Erros em programas científicos podem pôr em dúvida toda a pesquisa. Este artigo listará dezenas de problemas encontrados no código do pacote de software NCBI Genome Workbench.

1. Introdução


O NCBI Genome Workbench oferece aos pesquisadores uma ampla gama de ferramentas para estudar e analisar dados genéticos. Os usuários podem pesquisar e comparar dados de várias fontes, incluindo os bancos de dados do NCBI (National Center for Biotechnology Information) ou seus próprios dados pessoais.

Como mencionado anteriormente, o software científico geralmente é bem coberto por testes de unidade. Ao verificar este projeto, 85 diretórios com arquivos de teste foram excluídos da análise. São cerca de mil arquivos. Talvez isso se deva aos requisitos para testar vários algoritmos complexos que são inventados para vários estudos. Mas a qualidade do restante do código (não o teste) não está em um nível tão alto quanto gostaríamos. No entanto, como em qualquer projeto em que eles ainda não se encarregaram de introduzir ferramentas estáticas de análise de código :).

Os dados para revisão (ou mesmo pesquisa) do código foram fornecidos pelo analisador de código estático para C / C ++ / C # / Java - PVS-Studio .

Apenas dois números que arruinam seu projeto




Com base em nosso banco de dados de erros, que atualmente soma mais de 12 mil exemplos selecionados, observamos e descrevemos padrões específicos para escrever código que levam a inúmeros erros. Por exemplo, realizamos os seguintes estudos:

  1. O efeito da última linha ;
  2. A função mais perigosa do mundo do C / C ++ ;
  3. Expressões lógicas em C / C ++. Quão errados são os profissionais ;
  4. O mal vive em funções de comparação .

Este projeto marcou o início da descrição do novo padrão. Estamos falando dos números 1 e 2 nos nomes das variáveis, por exemplo, arquivo1 e arquivo2 , etc. É muito fácil confundir duas dessas variáveis. Este é um caso especial de erro de digitação no código, mas um desses erros resulta no desejo de trabalhar com variáveis ​​com o mesmo nome, que diferem apenas pelos números 1 e 2 no final do nome.

No futuro, direi que todos os estudos listados acima foram confirmados no código deste projeto: D.

Considere o primeiro exemplo do projeto Genome Workbench:

V501 Existem sub-expressões idênticas '(! Loc1.IsInt () &&! Loc1.IsWhole ())' à esquerda e à direita da '||' operador. 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"); } .... }

Vemos duas variáveis ​​nomeadas loc1 e loc2 . E também um erro no código: a variável loc2 não é usada, porque em vez disso, loc1 é usada novamente.

Outro exemplo:

V560 Uma parte da expressão condicional é sempre falsa: 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 { ..... }

A primeira linha de código misturou as variáveis s1 e s2 . Com base no nome, esta é uma função de comparação. Mas esse erro pode estar em qualquer lugar, porque, ao nomear as variáveis Número 1 e Número 2 , o programador quase certamente cometerá um erro no futuro. E quanto mais usos desses nomes em uma função, maior a probabilidade de um erro.

Outros erros de digitação e copiar e colar




V501 Existem subexpressões idênticas à esquerda e à direita do operador '! =': 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; } } .... }

Acredito que, após todas as verificações, os tamanhos das matrizes de bits dos objetos bd.bit_ e ib_db.bit_ são iguais. Portanto, o autor do código escreveu um ciclo para a comparação elementar de matrizes de bits , mas cometeu um erro de digitação no nome de um dos objetos comparados. Como resultado, objetos comparados podem ser equivocadamente considerados iguais em algumas situações.

Este exemplo é digno do artigo "O mal vive em funções de comparação ".

V501 Existem sub-expressões idênticas 'CFieldHandler :: QualifierNamesAreEquivalent (campo, kFieldTypeSeqId)' à esquerda e à direita da '||' operador. 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; } }

Provavelmente, uma das verificações é supérflua. Não encontrei nas variáveis ​​de código semelhantes ao kFieldTypeSeqId . No entanto, uma chamada de função extra é possível devido ao operador "||", que prejudica o desempenho.

Mais alguns do mesmo tipo de lugares com um aviso do analisador, exigindo verificação:

  • V501 Existem subexpressões idênticas 'uf-> GetData (). IsBool ()' à esquerda e à direita do operador '&&'. variação_utils.cpp 1711
  • V501 Existem subexpressões idênticas 'uf-> GetData (). IsBool ()' à esquerda e à direita do operador '&&'. variação_utils.cpp 1735

V766 Um item com a mesma chave 'kArgRemote' já foi adicionado. 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"); .... }

O analisador detectou a adição de 2 valores idênticos ao recipiente definido . Lembre-se de que esse contêiner armazena apenas valores exclusivos, portanto, duplicatas não são adicionadas a ele.

Código como o descrito acima é geralmente escrito usando o método copiar e colar. Pode haver simplesmente um valor extra, ou talvez o autor tenha esquecido de renomear uma das variáveis ​​ao copiar. Quando você remove uma chamada extra para inserir, o código é ligeiramente otimizado, o que, no entanto, não é significativo. Mais importante, um erro grave pode estar oculto aqui devido a um elemento ausente no conjunto.

V523 A instrução 'then' é equivalente ao fragmento de código subsequente. 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; }

A função contém fragmentos de código grandes e completamente idênticos. No entanto, eles contêm vários comentários que o acompanham. O código não foi escrito de maneira ideal, confusa e possivelmente contém um erro.

A lista inteira de lugares suspeitos com a instrução if-else é assim:

  • V523 A instrução 'then' é equivalente à instrução 'else'. blk.c 2142
  • V523 A instrução 'then' é equivalente ao fragmento de código subsequente. odbc.c 379
  • V523 A instrução 'then' é equivalente ao fragmento de código subsequente. odbc.c 1414
  • V523 A instrução 'then' é equivalente à instrução 'else'. seqdbvol.cpp 1922
  • V523 A instrução 'then' é equivalente à instrução 'else'. seqdb_demo.cpp 466
  • V523 A instrução 'then' é equivalente ao fragmento de código subsequente. blast_engine.c 1917
  • V523 A instrução 'then' é equivalente à instrução 'else'. blast_filter.c 420
  • V523 A instrução 'then' é equivalente à instrução 'else'. blast_parameters.c 636
  • V523 A instrução 'then' é equivalente à instrução 'else'. unordered_spliter.cpp 684
  • V523 A instrução 'then' é equivalente à instrução 'else'. bme.cpp 333
  • V523 A instrução 'then' é equivalente à instrução 'else'. gme.cpp 484

/ * com segurança é melhor ser pedante * /



V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'passwd_buf'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 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 { .... } }

Como você provavelmente já adivinhou, um comentário engraçado sobre a segurança do código foi usado no título da seção.

Em resumo, a função memset será removida pelo compilador, porque os buffers liberados não são mais usados. E dados como hash ou passwd_buf não serão realmente zeros. Para obter mais informações sobre esse mecanismo não óbvio do compilador, consulte o artigo " Limpeza segura de dados particulares ".

V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'responder'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 561

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

Esse não foi o único exemplo com comentários sobre "segurança". A julgar pelos comentários, pode-se supor que a segurança é realmente importante para o projeto. Portanto, estou anexando a lista não pequena inteira de problemas identificados:

  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'heap'. A função memset_s () deve ser usada para apagar os dados privados. ncbi_heapmgr.c 1300
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'contexto'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 167
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'ks'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 339
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'md5_ctx'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 353
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'hash'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 365
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'ks'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 406
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'ntlm_v2_response'. A função memset_s () deve ser usada para apagar os dados privados. login.c 795
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'responder'. A função memset_s () deve ser usada para apagar os dados privados. login.c 801
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer de 'pacote'. A função memset_s () deve ser usada para apagar os dados privados. numeric.c 256
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer de 'pacote'. A função memset_s () deve ser usada para apagar os dados privados. numeric.c 110
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'pwd'. A função memset_s () deve ser usada para apagar os dados privados. getpassarg.c 50
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'contexto'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 188
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'buf'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 243
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'ntlm_v2_hash'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 309
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'md5_ctx'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 354
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'passwd_buf'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 380
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'ks'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 393
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'hash'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 394
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'ntlm2_challenge'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 395
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'ks'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 419
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'ntlm_v2_response'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 556

Ciclos suspeitos




V534 É provável que uma variável incorreta esteja sendo comparada dentro do operador 'for'. Considere revisar '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; } .... } .... }

Eu acho que, na condição do loop interno, a variável que eu peguei aleatoriamente. Em vez disso, a variável j deve ser usada.

V535 A variável 'i' está sendo usada para esse loop e para o loop externo. Verifique as linhas: 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++) // <= { .... .... }

Dois ciclos idênticos aninhados, nos quais o contador global também é redefinido, parecem muito suspeitos. Os desenvolvedores devem verificar o que acontece aqui.

Indexação de matriz anormal




V520 O operador de vírgula ',' na expressão do índice de matriz '[- 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); } .... }

Devo dizer imediatamente que parece não haver erro (por enquanto, lol). Considere a seguinte linha:

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

A palavra 'matriz' e indexação dupla podem sugerir que a matriz é bidimensional, mas não é. Este é um ponteiro regular para uma matriz de números inteiros. Mas o diagnóstico do V520 não apareceu apenas. Os programadores estão realmente confusos sobre como indexar matrizes bidimensionais.

Nesse caso, o autor simplesmente decidiu salvar em uma linha de código, embora pudesse escrever o seguinte:

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

V661 Uma expressão suspeita 'A [B == C]'. Provavelmente significava '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*/; } .... }

Outro exemplo de código no qual passei muito tempo tentando entender o que estava acontecendo: D. A função isspace () verifica o caractere com o índice m , mas se esse caractere for '$', o caractere com o índice m + 1 é passado para a função. Além disso, a comparação com '$' já estava adiantada. Talvez não haja erro aqui, mas o código pode ser definitivamente reescrito com mais clareza.

A saturação da matriz V557 é possível. O índice de 'linha' está apontando além do limite da matriz. 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) { .... } .... }

Aqui há um erro grave. A verificação correta do índice de linha deve ser assim:

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

Caso contrário, é possível acessar dados fora do vetor MiddleSections .

Muitos mais desses lugares:

  • A saturação da matriz V557 é possível. O índice 'i' está apontando além do limite da matriz. resource_pool.hpp 388
  • A saturação da matriz V557 é possível. O índice de 'linha' está apontando além do limite da matriz. aln_reader.cpp 418
  • A saturação da matriz V557 é possível. O índice 'fmt_idx' está apontando além do limite da matriz. seq_writer.cpp 384
  • A saturação da matriz V557 é possível. O índice 'fmt_idx' está apontando além do limite da matriz. blastdb_formatter.cpp 183
  • A saturação da matriz V557 é possível. O índice 'num' está apontando além do limite da matriz. newcleanupp.cpp 13035

Como ganhar desconfiança nas funções




V570 A variável 'm_onClickFunction' é atribuída a si mesma. alngraphic.hpp 103

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

Não há nada para comentar. Você só pode simpatizar com a pessoa que clicou em algo, clicou, mas nada mudou.

Mais dois casos de atribuição de variáveis ​​para mim resultarão em uma lista:

  • V570 A variável 'iter-> level' é atribuída a si mesma. align_format_util.cpp 189
  • V570 A variável 'd_elements_values ​​[ind]' é atribuída a si mesma. sls_alp_data.cpp 1416

V763 O parâmetro 'w1' é sempre reescrito no corpo da função antes de ser usado. 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; } };

Uma função na qual o argumento é desgastado imediatamente após a entrada na função pode ser enganosa para os desenvolvedores que a utilizam. O código deve ser verificado duas vezes.

Erros de design de classe




V688 O argumento da função 'm_qsrc' possui o mesmo nome que um dos membros da classe, o que pode resultar em confusão. compart_matching.cpp 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); .... };

Imediatamente três funções de classe contêm argumentos cujos nomes coincidem com o campo de classe. Isso pode levar a erros nos corpos das funções: o programador pode pensar que está trabalhando com um membro da classe, alterando realmente o valor da variável local.

V614 Variável não inicializada 'm_BitSet' usada. 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); }

Um dos construtores trabalha desleixadamente com a variável m_BitSet . O fato é que a variável não foi inicializada. Seu valor de "lixo" é usado na primeira iteração do loop, após o qual a inicialização ocorre. Este é um erro muito sério, levando a um comportamento indefinido do programa.

V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> SIntervalComparisonResult :: SIntervalComparisonResult (....)' deve ser usado. 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); } .... };

Há muito tempo, não encontrei esses erros ao verificar projetos. Mas o problema ainda é relevante. O erro é que chamar o construtor parametrizado dessa maneira cria e remove um objeto temporário. E os campos da classe permanecem não inicializados. Outro construtor deve ser chamado através da lista de inicialização (consulte Delegando o construtor ).

V591 A função não nula deve retornar um valor. bio_tree.hpp 266

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

O analisador considera que a linha está ausente na instrução sobrecarregada:

 return *this;

V670 O membro da classe não inicializado 'm_OutBlobIdOrData' é usado para inicializar o membro 'm_StdOut'. Lembre-se de que os membros são inicializados na ordem de suas declarações dentro de uma classe. 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 avisos do analisador são emitidos imediatamente para este fragmento de código. Os campos de classe são inicializados não na ordem em que estão listados na lista de inicialização, mas na maneira como são declarados na classe. A causa clássica do erro é que nem todos os programadores se lembram ou conhecem essa regra. Aqui e na lista de inicialização está a ordem errada. Ficamos com a sensação de que a lista de campos foi inserida em ordem aleatória.

V746 Corte de objetos. Uma exceção deve ser capturada por referência e não por valor. cobalt.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); .... }

A captura de exceções por valor pode levar à perda de algumas informações sobre a exceção devido à criação de um novo objeto. É muito melhor e mais seguro capturar uma exceção por referência.

Locais similares:

  • V746 Corte de objetos. Uma exceção deve ser capturada por referência e não por valor. agp_validate_reader.cpp 562
  • V746 Corte de objetos. Uma exceção deve ser capturada por referência e não por valor. aln_build_app.cpp 320
  • V746 Corte de objetos. Uma exceção deve ser capturada por referência e não por valor. aln_test_app.cpp 458
  • V746 Corte de objetos. Uma exceção deve ser capturada por referência e não por valor. cobalt.cpp 691
  • V746 Corte de objetos. Uma exceção deve ser capturada por referência e não por valor. cobalt.cpp 719
  • V746 Corte de objetos. Uma exceção deve ser capturada por referência e não por valor. cobalt.cpp 728
  • V746 Corte de objetos. Uma exceção deve ser capturada por referência e não por valor. cobalt.cpp 732

Sobre código inacessível e outros problemas de execução de código



V779 Código inacessível detectado. É possível que haja um erro. 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; }

O código da instrução condicional é escrito para que absolutamente todas as ramificações do código terminem com a instrução continue . Isso levou a várias linhas de código inacessível que se formaram no loop while . Essas linhas parecem muito suspeitas. Muito provavelmente, esse problema surgiu após a refatoração do código e agora exige uma revisão cuidadosa do código.

V519 A variável 'interval_width' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 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; .... } .... }

A variável interval_width é substituída várias vezes, porque não há instruções de interrupção nos ramos do caso . Embora seja um erro clássico, mas muito ruim.

Mais alguns lugares suspeitos:

  • V779 Código inacessível detectado. É possível que haja um erro. dbapi_driver_utils.cpp 351
  • V779 Código inacessível detectado. É possível que haja um erro. net.c 780
  • V779 Código inacessível detectado. É possível que haja um erro. bcp.c 1495
  • V779 Código inacessível detectado. É possível que haja um erro. remote_blast.cpp 1470
  • V779 Código inacessível detectado. É possível que haja um erro. remote_blast.cpp 1522

V571 Verificação recorrente. A condição 'if (m_QueryOpts-> filtering_options)' já foi verificada na linha 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; } .... }

Obviamente, o ramo else requer reescrita. Eu tenho algumas idéias que eu queria fazer com o ponteiro m_QueryOpts-> filtering_options , mas o código ainda é de alguma forma confuso. Apelo aos autores do código.

Bem, o problema não vem sozinho:

  • V571 Verificação recorrente. A condição 'if (sleeptime)' já foi verificada na linha 205. request_control.cpp 208
  • V571 Verificação recorrente. A condição 'if (assignValue.empty ())' já foi verificada na linha 712. classstr.cpp 718

Erros de leitura de dados



O V739 EOF não deve ser comparado com um valor do tipo 'char'. O 'linestring [0]' deve ser do tipo 'int'. alnread.c 3509

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

Os caracteres que você planeja comparar com o EOF não devem ser armazenados em variáveis ​​de caracteres . Caso contrário, existe o risco de um caractere com o valor 0xFF (255) se transformar em -1 e ser interpretado da mesma maneira que o final de um arquivo (EOF). Também (apenas no caso) vale a pena verificar a implementação da função readfunc .

V663 Loop infinito é possível. A condição 'cin.eof ()' é insuficiente para interromper o loop.Considere adicionar a chamada de função 'cin.fail ()' à expressão condicional. 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()); } } }

O analisador detectou um erro em potencial devido ao qual um loop infinito pode ocorrer. Se ocorrer uma falha durante a leitura dos dados, chamar a função eof () sempre retornará falso . Para concluir o loop nesse caso, é necessária uma verificação adicional do valor retornado pela função fail () .

Erros diversos



V502 Talvez o operador '?:' Funcione de maneira diferente do esperado. O operador '?:' Tem uma prioridade mais baixa que o operador '&&'. 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 :-/*/; } .... }

Preste atenção à expressão:

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


Não é calculado como o programador esperava, porque a expressão inteira se parece com isso:

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


A prioridade do operador && é maior que ?: . Por esse motivo, o código não é executado conforme o esperado.

V561 Provavelmente é melhor atribuir valor à variável 'seq' do que declará-lo novamente. Declaração anterior: validator.cpp, linha 490. validator.cpp 492

 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); }

Devido ao programador declarar uma nova variável seq dentro da seção try / catch, a outra variável seq permanece não inicializada e é usada abaixo no código.

V562 É estranho comparar um valor do tipo bool com um valor de 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 }

Nada deu errado, mas o WIFEXITED acabou sendo uma abertura macro desta maneira:

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

Acontece que a função retorna o valor oposto.

No código, havia outra função desse tipo, que emitia um aviso:

  • V562 É estranho comparar um valor do tipo bool com um valor 0. ncbi_process.cpp 126

V595 O ponteiro 'dst_len' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 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; } .... }

O ponteiro dst_len é desreferenciado no início da função, enquanto mais adiante o código é verificado para que a igualdade seja zero. Um erro foi cometido no código que leva a um comportamento indefinido se o ponteiro dst_len for nullptr .

V590 Considere inspecionar a expressão 'ch! =' \ 0 '&& ch ==' ''. A expressão é excessiva ou contém uma impressão incorreta. cleanup_utils.cpp 580

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

A condição para interromper o loop depende apenas de o caracter ch ser um espaço ou não. A expressão pode ser simplificada para o seguinte:

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

Conclusão


O uso de programas de computador em pesquisas científicas ajuda e ajudará a fazer descobertas. Vamos esperar que os mais importantes não sejam perdidos devido a erros de digitação.

Convido os desenvolvedores do projeto NCBI Genome Workbench a entrar em contato conosco e forneceremos um relatório completo emitido pelo analisador PVS-Studio.

Espero que esta pequena pesquisa de código ajude a corrigir muitos erros e, geralmente, a melhorar a confiabilidade do projeto. Tente executar o PVS-Studio no código de seus projetos, se você ainda não o fez. Você pode gostar :).



Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Svyatoslav Razmyslov. NCBI Genome Workbench: pesquisa científica sob ameaça

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

More articles:


All Articles