Geekly articles weekly

NCBI Genome Workbench: Investigación en peligro de extinción

Tecnologías informáticas modernas, soluciones técnicas y de software: todo esto facilita y acelera enormemente la implementación de diversas investigaciones científicas. A menudo, la simulación por computadora es la única forma de probar muchas teorías. El software científico tiene sus propias características. Por ejemplo, dicho software a menudo se somete a pruebas muy exhaustivas, pero está poco documentado. Sin embargo, el software está escrito por personas y las personas cometen errores. Los errores en los programas científicos pueden poner en duda toda la investigación. Este artículo enumerará docenas de problemas encontrados en el código del paquete de software NCBI Genome Workbench.

Introduccion


El NCBI Genome Workbench ofrece a los investigadores una amplia gama de herramientas para estudiar y analizar datos genéticos. Los usuarios pueden investigar y comparar datos de varias fuentes, incluidas las bases de datos del NCBI (Centro Nacional de Información Biotecnológica) o sus propios datos personales.

Como se mencionó anteriormente, el software científico generalmente está bien cubierto por las pruebas unitarias. Al verificar este proyecto, 85 directorios con archivos de prueba fueron excluidos del análisis. Esto es alrededor de mil archivos. Quizás esto se deba a los requisitos para probar varios algoritmos complejos que se inventan para varios estudios. Pero la calidad del resto del código (no el de prueba) no está en un nivel tan alto como nos gustaría. Sin embargo, como en cualquier proyecto en el que aún no se hayan ocupado de introducir herramientas de análisis de código estático :).

Los datos para la revisión (o incluso la investigación) del código fueron proporcionados por el analizador de código estático para C / C ++ / C # / Java - PVS-Studio .

Solo dos números que arruinan tu proyecto




Con base en nuestra base de datos de errores, que actualmente asciende a más de 12 mil ejemplos seleccionados, notamos y describimos patrones específicos para escribir código que conducen a numerosos errores. Por ejemplo, realizamos los siguientes estudios:

  1. El efecto de la última línea ;
  2. La función más peligrosa en el mundo de C / C ++ ;
  3. Expresiones lógicas en C / C ++. Qué equivocados están los profesionales ;
  4. El mal vive en funciones de comparación .

Este proyecto marcó el comienzo de la descripción del nuevo patrón. Estamos hablando de los números 1 y 2 en los nombres de variables, por ejemplo, archivo1 y archivo2 , etc. Es muy fácil confundir dos de esas variables. Este es un caso especial de errores tipográficos en el código, pero uno de esos errores da como resultado el deseo de trabajar con variables del mismo nombre, que difieren solo por los números 1 y 2 al final del nombre.

Mirando hacia el futuro un poco, diré que todos los estudios enumerados anteriormente se confirmaron en el código de este proyecto: D.

Considere el primer ejemplo del proyecto Genome Workbench:

V501 Hay subexpresiones idénticas '(! Loc1.IsInt () &&! Loc1.IsWhole ())' a la izquierda y a la derecha de '||' 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 dos variables llamadas loc1 y loc2 . Y también un error en el código: la variable loc2 no se usa, porque en su lugar se usa loc1 una vez más.

Otro ejemplo:

V560 Una parte de la expresión condicional siempre es 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 { ..... }

La primera línea de código mezcló las variables s1 y s2 . Según el nombre, esta es una función de comparación. Pero tal error puede estar en cualquier parte, porque al nombrar las variables Número 1 y Número 2 , el programador seguramente cometerá un error en el futuro. Y cuanto más usos de tales nombres en una función, mayor es la probabilidad de un error.

Otros errores tipográficos y copiar y pegar




V501 Hay subexpresiones idénticas a la izquierda y a la derecha del 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; } } .... }

Creo que después de todas las comprobaciones, los tamaños de las matrices de bits de los objetos bd.bit_ e ib_db.bit_ son iguales. Por lo tanto, el autor del código escribió un ciclo para la comparación por elementos de las matrices de bits , pero hizo un error tipográfico en el nombre de uno de los objetos comparados. Como resultado, los objetos comparados pueden considerarse erróneamente iguales en algunas situaciones.

Este ejemplo es digno del artículo "El mal vive en funciones de comparación ".

V501 Hay subexpresiones idénticas 'CFieldHandler :: QualifierNamesAreEquivalent (field, kFieldTypeSeqId)' a la izquierda y a la derecha de '||' 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; } }

Lo más probable es que uno de los controles sea superfluo. No encontré en el código variables similares a kFieldTypeSeqId . Sin embargo, es posible una llamada de función adicional debido al operador "||", que degrada el rendimiento.

Un par más del mismo tipo de lugares con una advertencia del analizador, que requieren verificación:

  • V501 Hay subexpresiones idénticas 'uf-> GetData (). IsBool ()' a la izquierda y a la derecha del operador '&&'. variación_utils.cpp 1711
  • V501 Hay subexpresiones idénticas 'uf-> GetData (). IsBool ()' a la izquierda y a la derecha del operador '&&'. variación_utils.cpp 1735

V766 Ya se ha agregado un elemento con la misma clave '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"); .... }

El analizador detectó la adición de 2 valores idénticos al contenedor establecido . Recuerde que este contenedor solo almacena valores únicos, por lo que no se le agregan duplicados.

El código como el anterior a menudo se escribe utilizando el método de copiar y pegar. Simplemente puede haber un valor adicional, o tal vez el autor olvidó cambiar el nombre de una de las variables cuando copió. Cuando elimina una llamada adicional para insertar, el código se optimiza ligeramente, lo que, sin embargo, no es significativo. Más importante aún, un error grave puede estar oculto aquí debido a un elemento faltante en el conjunto.

V523 La declaración 'then' es equivalente al fragmento de código posterior. 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; }

La función contiene fragmentos de código grandes y completamente idénticos. Sin embargo, contienen varios comentarios acompañantes. El código no está escrito de manera óptima, confusa, y posiblemente contiene un error.

La lista completa de lugares sospechosos con la declaración if-else se ve así:

  • V523 La declaración 'then' es equivalente a la declaración 'else'. blk.c 2142
  • V523 La declaración 'then' es equivalente al fragmento de código posterior. odbc.c 379
  • V523 La declaración 'then' es equivalente al fragmento de código posterior. odbc.c 1414
  • V523 La declaración 'then' es equivalente a la declaración 'else'. seqdbvol.cpp 1922
  • V523 La declaración 'then' es equivalente a la declaración 'else'. seqdb_demo.cpp 466
  • V523 La declaración 'then' es equivalente al fragmento de código posterior. blast_engine.c 1917
  • V523 La declaración 'then' es equivalente a la declaración 'else'. blast_filter.c 420
  • V523 La declaración 'then' es equivalente a la declaración 'else'. blast_parameters.c 636
  • V523 La declaración 'then' es equivalente a la declaración 'else'. unordered_spliter.cpp 684
  • V523 La declaración 'then' es equivalente a la declaración 'else'. bme.cpp 333
  • V523 La declaración 'then' es equivalente a la declaración 'else'. gme.cpp 484

/ * con seguridad es mejor ser pedante * /



V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'passwd_buf'. La función memset_s () debe usarse para borrar los datos 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 probablemente ya adivinó, en el título de la sección se utilizó un comentario divertido sobre la seguridad del código.

En resumen, la función memset será eliminada por el compilador, porque los buffers vaciados ya no se utilizan. Y datos como hash o passwd_buf en realidad no serán ceros. Para obtener más información sobre este mecanismo de compilación no obvio, consulte el artículo " Limpieza segura de datos privados ".

V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'responder'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 561

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

Ese no fue el único ejemplo con comentarios sobre "seguridad". A juzgar por los comentarios, se puede suponer que la seguridad es realmente importante para el proyecto. Por lo tanto, adjunto toda la lista no pequeña de problemas identificados:

  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el objeto 'montón'. La función memset_s () debe usarse para borrar los datos privados. ncbi_heapmgr.c 1300
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'context'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 167
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'ks'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 339
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'md5_ctx'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 353
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'hash'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 365
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'ks'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 406
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el objeto 'ntlm_v2_response'. La función memset_s () debe usarse para borrar los datos privados. login.c 795
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'responder'. La función memset_s () debe usarse para borrar los datos privados. login.c 801
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'paquete'. La función memset_s () debe usarse para borrar los datos privados. numeric.c 256
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'paquete'. La función memset_s () debe usarse para borrar los datos privados. numeric.c 110
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'pwd'. La función memset_s () debe usarse para borrar los datos privados. getpassarg.c 50
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'context'. La función memset_s () debe usarse para borrar los datos privados. desafío.c 188
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'buf'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 243
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'ntlm_v2_hash'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 309
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'md5_ctx'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 354
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'passwd_buf'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 380
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'ks'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 393
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'hash'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 394
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'ntlm2_challenge'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 395
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'ks'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 419
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el objeto 'ntlm_v2_response'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 556

Ciclos sospechosos




V534 Es probable que se compare una variable incorrecta dentro del 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; } .... } .... }

Creo que, en la condición del bucle interno, la variable la obtuve al azar. En cambio, se debe usar la variable j .

V535 La variable 'i' se está utilizando para este bucle y para el bucle externo. Líneas de verificación: 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++) // <= { .... .... }

Dos ciclos idénticos anidados, en los que el contador global también se restablece, parecen muy sospechosos. Los desarrolladores deben verificar lo que sucede aquí.

Indización de matriz anormal




V520 El operador de coma ',' en la expresión de í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); } .... }

Debo decir de inmediato que parece no haber ningún error (por ahora, jajaja). Considere la siguiente línea:

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

La palabra 'matriz' y la doble indexación pueden sugerir que la matriz es bidimensional, pero no lo es. Este es un puntero regular a una matriz de enteros. Pero el diagnóstico V520 no solo apareció. Los programadores están realmente confundidos acerca de cómo indexar matrices bidimensionales.

En este caso, el autor simplemente decidió guardar en una línea de código, aunque podría escribir así:

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

V661 Una expresión sospechosa 'A [B == C]'. Probablemente significaba '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*/; } .... }

Otro ejemplo de código en el que pasé mucho tiempo tratando de entender lo que está sucediendo: D. La función isspace () verifica el carácter con índice m , pero si este carácter es '$', entonces el carácter con índice m + 1 se pasa a la función. Además, la comparación con '$' ya estaba por adelantado. Quizás no haya ningún error aquí, pero el código definitivamente se puede reescribir más claramente.

V557 Array overrun es posible. El índice de 'fila' apunta más allá del límite de la 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) { .... } .... }

Aquí hay un grave error. La verificación correcta del índice de fila debería ser así:

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

De lo contrario, es posible acceder a datos fuera del vector de MiddleSections .

Muchos más de esos lugares:

  • V557 Array overrun es posible. El índice 'i' apunta más allá del límite de la matriz. resource_pool.hpp 388
  • V557 Array overrun es posible. El índice de 'fila' apunta más allá del límite de la matriz. aln_reader.cpp 418
  • V557 Array overrun es posible. El índice 'fmt_idx' apunta más allá del límite de la matriz. seq_writer.cpp 384
  • V557 Array overrun es posible. El índice 'fmt_idx' apunta más allá del límite de la matriz. blastdb_formatter.cpp 183
  • V557 Array overrun es posible. El índice 'num' apunta más allá del límite de la matriz. newcleanupp.cpp 13035

Cómo ganarse la desconfianza de las funciones




V570 La variable 'm_onClickFunction' se asigna a sí misma. alngraphic.hpp 103

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

No hay nada que comentar. Solo puede simpatizar con la persona que hizo clic en algo, hizo clic, pero nada cambió.

Dos casos más de asignarme variables darán como resultado una lista:

  • V570 La variable 'iter-> level' se asigna a sí misma. align_format_util.cpp 189
  • V570 La variable 'd_elements_values ​​[ind]' se asigna a sí misma. sls_alp_data.cpp 1416

V763 El parámetro 'w1' siempre se reescribe en el cuerpo de la función antes de usarse. 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; } };

Una función en la que el argumento se deshilacha inmediatamente al ingresar a la función puede ser engañoso para los desarrolladores que la usan. El código debe ser verificado dos veces.

Errores de diseño de clase




V688 El argumento de la función 'm_qsrc' posee el mismo nombre que uno de los miembros de la clase, lo que puede generar confusión. 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); .... };

Inmediatamente 3 funciones de clase contienen argumentos cuyos nombres coinciden con el campo de clase. Esto puede conducir a errores en los cuerpos de las funciones: el programador puede pensar que está trabajando con un miembro de la clase, cambiando realmente el valor de la variable local.

V614 Variable no inicializada 'm_BitSet' utilizada. 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); }

Uno de los constructores trabaja descuidadamente con la variable m_BitSet . El hecho es que la variable no está inicializada. Su valor de "basura" se utiliza en la primera iteración del bucle, después de lo cual ocurre la inicialización. Este es un error muy grave, que conduce a un comportamiento indefinido del programa.

V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar '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); } .... };

Hace mucho tiempo no encontré tales errores al verificar proyectos. Pero el problema sigue siendo relevante. El error es que llamar al constructor parametrizado de esta manera crea y elimina un objeto temporal. Y los campos de clase permanecen sin inicializar. Se debe llamar a otro constructor a través de la lista de inicialización (consulte Delegar el constructor ).

V591 La función no nula debería devolver un valor. bio_tree.hpp 266

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

El analizador considera que falta la línea en la declaración sobrecargada:

 return *this;

V670 El miembro de clase no inicializado 'm_OutBlobIdOrData' se usa para inicializar el miembro 'm_StdOut'. Recuerde que los miembros se inicializan en el orden de sus declaraciones dentro de una clase. 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) { } .... };

Se emiten inmediatamente 3 advertencias del analizador a este fragmento de código. Los campos de clase se inicializan no en el orden en que aparecen en la lista de inicialización, sino en la forma en que se declaran en la clase. La causa clásica del error es que no todos los programadores recuerdan o conocen esta regla. Aquí y en la lista de inicialización está el orden incorrecto. Uno tiene la sensación de que la lista de campos se ingresó en orden aleatorio.

V746 Rebanado de objetos. Se debe detectar una excepción por referencia en lugar de 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); .... }

Capturar excepciones por valor puede conducir a la pérdida de cierta información sobre la excepción debido a la creación de un nuevo objeto. Es mucho mejor y más seguro detectar una excepción por referencia.

Lugares similares:

  • V746 Rebanado de objetos. Se debe detectar una excepción por referencia en lugar de por valor. agp_validate_reader.cpp 562
  • V746 Rebanado de objetos. Se debe detectar una excepción por referencia en lugar de por valor. aln_build_app.cpp 320
  • V746 Rebanado de objetos. Se debe detectar una excepción por referencia en lugar de por valor. aln_test_app.cpp 458
  • V746 Rebanado de objetos. Se debe detectar una excepción por referencia en lugar de por valor. cobalt.cpp 691
  • V746 Rebanado de objetos. Se debe detectar una excepción por referencia en lugar de por valor. cobalt.cpp 719
  • V746 Rebanado de objetos. Se debe detectar una excepción por referencia en lugar de por valor. cobalt.cpp 728
  • V746 Rebanado de objetos. Se debe detectar una excepción por referencia en lugar de por valor. cobalt.cpp 732

Acerca del código inalcanzable y otros problemas de ejecución del código



V779 Código inalcanzable detectado. Es posible que haya un error presente. 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; }

El código de la declaración condicional se escribe de modo que absolutamente todas las ramas del código terminen con la declaración de continuación . Esto condujo a varias líneas de código inalcanzable que se forman en el ciclo while . Estas líneas se ven muy sospechosas. Lo más probable es que este problema surgió después de refactorizar el código, y ahora requiere una revisión cuidadosa del código.

V519 La variable 'interval_width' tiene valores asignados dos veces sucesivamente. Quizás esto sea un error. Líneas de verificación: 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; .... } .... }

La variable interval_width se sobrescribe varias veces, porque No hay declaraciones de ruptura en las ramas de casos . Aunque es un clásico, pero es un error muy malo.

Algunos lugares más sospechosos:

  • V779 Código inalcanzable detectado. Es posible que haya un error presente. dbapi_driver_utils.cpp 351
  • V779 Código inalcanzable detectado. Es posible que haya un error presente. net.c 780
  • V779 Código inalcanzable detectado. Es posible que haya un error presente. bcp.c 1495
  • V779 Código inalcanzable detectado. Es posible que haya un error presente. remote_blast.cpp 1470
  • V779 Código inalcanzable detectado. Es posible que haya un error presente. remote_blast.cpp 1522

V571 Verificación recurrente. La condición 'if (m_QueryOpts-> filtering_options)' ya se verificó en la línea 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, la rama else requiere reescritura. Tengo algunas ideas que quería hacer con el puntero m_QueryOpts-> filtering_options , pero el código sigue siendo algo confuso. Apelo a los autores del código.

Bueno, el problema no viene solo:

  • V571 Verificación recurrente. La condición 'if (sueño)' ya se verificó en la línea 205. request_control.cpp 208
  • V571 Verificación recurrente. La condición 'if (asignarValor.empty ())' ya se verificó en la línea 712. classstr.cpp 718

Errores de lectura de datos



V739 EOF no debe compararse con un valor del tipo 'char'. La 'cadena de línea [0]' debe ser del tipo 'int'. alnread.c 3509

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

Los caracteres que planea comparar con EOF no deben almacenarse en variables char . De lo contrario, existe el riesgo de que un carácter con el valor 0xFF (255) se convierta en -1 y se interprete de la misma manera que el final de un archivo (EOF). Además (por si acaso) vale la pena verificar la implementación de la función readfunc .

V663 Infinite loop es posible. La condición 'cin.eof ()' es insuficiente para romper el bucle.Considere agregar la llamada a la función 'cin.fail ()' a la expresión 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()); } } }

El analizador detectó un error potencial debido al cual puede ocurrir un bucle infinito. Si ocurre una falla mientras lee datos, llamar a la función eof () siempre devolverá falso . Para completar el ciclo en este caso, es necesaria una verificación adicional del valor devuelto por la función fail () .

Errores varios



V502 Quizás el operador '?:' Funciona de una manera diferente a la esperada. El operador '?:' Tiene una prioridad menor que el 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 :-/*/; } .... }

Presta atención a la expresión:

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


No se calcula como esperaba el programador, porque la expresión completa se ve así:

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


La prioridad del operador && es mayor que ?: . Por esta razón, el código no se ejecuta según lo previsto.

V561 Probablemente sea mejor asignar valor a la variable 'seq' que declararlo de nuevo. Declaración previa: validator.cpp, línea 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); }

Debido a que el programador declara una nueva variable seq dentro de la sección try / catch, otra variable seq permanece sin inicializar y se usa a continuación en el código.

V562 Es extraño comparar un valor de tipo bool con un valor de 0: (((estado) & 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 anunciaba mal, pero WIFEXITED resultó ser una apertura macro de esta manera:

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

Resulta que la función devuelve el valor opuesto.

En el código, había otra función, que emitió una advertencia:

  • V562 Es extraño comparar un valor de tipo bool con un valor de 0. ncbi_process.cpp 126

V595 El puntero 'dst_len' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 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; } .... }

El puntero dst_len se desreferencia al comienzo de la función, mientras que más adelante se verifica que el código sea igual a cero. Se ha cometido un error en el código que conduce a un comportamiento indefinido si el puntero dst_len es nullptr .

V590 Considere inspeccionar la expresión 'ch! =' \ 0 '&& ch ==' ''. La expresión es excesiva o contiene un error de imprenta. cleanup_utils.cpp 580

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

La condición para detener el bucle depende solo de si el carácter ch es un espacio o no. La expresión se puede simplificar a lo siguiente:

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

Conclusión


El uso de programas de computadora en la investigación científica ayuda y ayudará a hacer descubrimientos. Esperemos que no se pierdan los especialmente importantes debido a algún error tipográfico.

Invito a los desarrolladores del proyecto NCBI Genome Workbench a contactarnos y les proporcionaremos un informe completo emitido por el analizador PVS-Studio.

Espero que esta pequeña investigación de código ayude a corregir muchos errores y, en general, mejore la confiabilidad del proyecto. Intente ejecutar PVS-Studio en el código de sus proyectos, si aún no lo ha hecho. Te puede gustar :).



Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Svyatoslav Razmyslov. NCBI Genome Workbench: Investigación científica bajo amenaza

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

More articles:


All Articles