LibreOffice es una poderosa suite ofimática que es gratuita para uso privado, educativo y comercial. Sus desarrolladores están haciendo un producto maravilloso, que en muchas áreas se usa como una alternativa a Microsoft Office. El equipo de PVS-Studio siempre está interesado en mirar el código de proyectos tan conocidos y tratar de encontrar errores en ellos. Esta vez fue fácil de hacer. El proyecto contiene muchos errores que pueden provocar problemas graves. El artículo discutirá algunos defectos interesantes encontrados en el código.
Introduccion
LibreOffice es un proyecto C ++ muy grande. Mantener un proyecto de este tamaño es una tarea difícil para el equipo de desarrollo. Y, desafortunadamente, uno tiene la impresión de que la calidad del código de LibreOffice no presta suficiente atención.
Por un lado, el proyecto es simplemente enorme, no todas las herramientas de análisis estático o dinámico pueden manejar el análisis de archivos de código fuente de 13k. Hay tantos archivos involucrados en la construcción de la suite ofimática junto con bibliotecas de terceros. El repositorio principal de LibreOffice almacena aproximadamente 8k archivos de código fuente. Esta cantidad de código crea problemas no solo para los desarrolladores:
Por otro lado, el proyecto tiene muchos usuarios y necesita encontrar y corregir tantos errores como sea posible. Cada error puede dañar a cientos y miles de usuarios. Por lo tanto, el gran tamaño de la base de código no debería convertirse en una excusa para negarse a usar ciertas herramientas que pueden detectar errores. Creo que el lector ya ha adivinado que estamos hablando de analizadores de código estático :).
Sí, el uso de analizadores estáticos no garantiza la ausencia de errores en el proyecto. Sin embargo, herramientas como PVS-Studio pueden encontrar una gran cantidad de errores en la etapa de desarrollo y, por lo tanto, reducen la cantidad de trabajo asociado con la depuración y el soporte del proyecto.
Veamos qué puede encontrar interesante en el código fuente de LibreOffice si toma el analizador de código estático
PVS-Studio . Las posibilidades para ejecutar el analizador son amplias: Windows, Linux, macOS. Para escribir esta revisión, se utilizó el informe PVS-Studio, creado durante el análisis del proyecto en Windows.
Cambios desde la última verificación en 2015
En marzo de 2015, se realizó el primer análisis de LibreOffice ("
Verificación del proyecto LibreOffice ") utilizando PVS-Studio. Desde entonces, la suite ofimática ha evolucionado mucho como producto, pero en su interior también contiene muchos errores. Y algunos patrones de error no han cambiado en absoluto desde entonces. Aquí, por ejemplo, es un error del primer artículo:
Las variables
V656 'aVRP', 'aVPN' se inicializan a través de la llamada a la misma función. Probablemente sea un error o un código no optimizado. Considere inspeccionar la expresión 'rSceneCamera.GetVRP ()'. Líneas de verificación: 177, 178. viewcontactofe3dscene.cxx 178
void ViewContactOfE3dScene::createViewInformation3D(....) { .... const basegfx::B3DPoint aVRP(rSceneCamera.GetVRP()); const basegfx::B3DVector aVPN(rSceneCamera.GetVRP());
Este error se ha solucionado, pero esto es lo que se encontró en la última versión del código:
V656 Las variables 'aSdvURL', 'aStrURL' se inicializan a través de la llamada a la misma función. Probablemente sea un error o un código no optimizado. Considere inspeccionar la expresión 'pThm-> GetSdvURL ()'. Líneas de verificación: 658, 659. gallery1.cxx 659
const INetURLObject& GetThmURL() const { return aThmURL; } const INetURLObject& GetSdgURL() const { return aSdgURL; } const INetURLObject& GetSdvURL() const { return aSdvURL; } const INetURLObject& GetStrURL() const { return aStrURL; } bool Gallery::RemoveTheme( const OUString& rThemeName ) { .... INetURLObject aThmURL( pThm->GetThmURL() ); INetURLObject aSdgURL( pThm->GetSdgURL() ); INetURLObject aSdvURL( pThm->GetSdvURL() ); INetURLObject aStrURL( pThm->GetSdvURL() );
Como habrás notado, los nombres de funciones compuestas sutiles siguen siendo una fuente de errores.
Otro ejemplo interesante del antiguo código:
Las variables
V656 'nDragW', 'nDragH' se inicializan a través de la llamada a la misma función. Probablemente sea un error o un código no optimizado. Considere inspeccionar la expresión 'rMSettings.GetStartDragWidth ()'. Líneas de verificación: 471, 472. winproc.cxx 472
class VCL_DLLPUBLIC MouseSettings { .... long GetStartDragWidth() const; long GetStartDragHeight() const; .... } bool ImplHandleMouseEvent( .... ) { .... long nDragW = rMSettings.GetStartDragWidth(); long nDragH = rMSettings.GetStartDragWidth(); .... }
Este fragmento de código realmente contenía un error que ahora está solucionado. Pero los errores en el código no se están reduciendo ... Ahora se ha identificado una situación similar:
Las variables
V656 'defaultZoomX', 'defaultZoomY' se inicializan a través de la llamada a la misma función. Probablemente sea un error o un código no optimizado. Considere inspeccionar la expresión 'pViewData-> GetZoomX ()'. Líneas de verificación: 5673, 5674. gridwin.cxx 5674
OString ScGridWindow::getCellCursor(....) const { .... SCCOL nX = pViewData->GetCurX(); SCROW nY = pViewData->GetCurY(); Fraction defaultZoomX = pViewData->GetZoomX(); Fraction defaultZoomY = pViewData->GetZoomX();
Los errores se introducen en el código literalmente por analogía.
No te dejes engañar
V765 Una expresión de asignación compuesta 'x - = x - ...' es sospechosa. Considere inspeccionarlo por un posible error. swdtflvr.cxx 3509
bool SwTransferable::PrivateDrop(...) { .... if ( rSrcSh.IsSelFrameMode() ) {
Aquí hay un "Hack" tan interesante que se encontró usando el diagnóstico
V765 . Si simplifica una línea de código con un comentario, puede obtener un resultado inesperado:
1er paso:
aSttPt = aSttPt - (aSttPt - rSrcSh.GetObjRect().Pos());
2do paso:
aSttPt = aSttPt - aSttPt + rSrcSh.GetObjRect().Pos();
3er paso
aSttPt = rSrcSh.GetObjRect().Pos();
¿Y entonces qué es hackear?
Otro ejemplo sobre este tema:
V567 La modificación de la variable 'nCount' no está secuenciada en relación con otra operación en la misma variable. Esto puede conducir a un comportamiento indefinido. stgio.cxx 214
FatError EasyFat::Mark(....) { if( nCount > 0 ) { --nCount /= GetPageSize(); nCount++; } .... }
La ejecución de código en tales situaciones puede depender del compilador y el estándar de idioma. ¿Por qué no reescribir este fragmento de código de una manera más simple, clara y confiable?
Cómo no usar matrices y vectores
Por alguna razón, alguien cometió muchos errores similares al trabajar con matrices y vectores. Veamos estos ejemplos.
V557 Array overrun es posible. El índice 'nPageNum' apunta más allá de la matriz. pptx-epptooxml.cxx 1168
void PowerPointExport::ImplWriteNotes(sal_uInt32 nPageNum) { ....
El último índice válido debe ser un valor igual a
size () - 1 . Pero en este fragmento de código, se permitió una situación en la que el índice
nPageNum puede tener el valor
mpSlidesFSArray.size () , por lo que hay una matriz fuera de los límites y funciona con un elemento que consiste en "basura".
V557 Array overrun es posible. El índice 'mnSelectedMenu' apunta más allá del límite de la matriz. checklistmenu.cxx 826
void ScMenuFloatingWindow::ensureSubMenuNotVisible() { if (mnSelectedMenu <= maMenuItems.size() && maMenuItems[mnSelectedMenu].mpSubMenuWin && maMenuItems[mnSelectedMenu].mpSubMenuWin->IsVisible()) { maMenuItems[mnSelectedMenu].mpSubMenuWin->ensureSubMenuNotVisible(); } EndPopupMode(); }
Curiosamente, en este fragmento de código escribieron una verificación de índice más claramente, pero al mismo tiempo cometieron el mismo error.
V557 Array overrun es posible. El índice 'nXFIndex' apunta más allá de la matriz. xestyle.cxx 2613
sal_Int32 XclExpXFBuffer::GetXmlStyleIndex( sal_uInt32 nXFIndex ) const { OSL_ENSURE( nXFIndex < maStyleIndexes.size(), "...." ); if( nXFIndex > maStyleIndexes.size() ) return 0;
¡Y este error es doblemente interesante! En la macro de depuración, escribieron la verificación de índice correcta, y en otro lugar nuevamente cometieron un error, permitiéndoles salir del conjunto.
Ahora considere un tipo diferente de error que no está relacionado con los índices.
V554 Uso incorrecto de shared_ptr. La memoria asignada con 'nuevo []' se limpiará con 'eliminar'. dx_vcltools.cxx 158
struct RawRGBABitmap { sal_Int32 mnWidth; sal_Int32 mnHeight; std::shared_ptr< sal_uInt8 > mpBitmapData; }; RawRGBABitmap bitmapFromVCLBitmapEx( const ::BitmapEx& rBmpEx ) { ....
Este fragmento de código contiene un error que conduce a un comportamiento indefinido del programa. El hecho es que la memoria se asigna y se libera de diferentes maneras. Para liberar memoria correctamente, tenía que declarar un campo de clase como este:
std::shared_ptr< sal_uInt8[] > mpBitmapData;
Cómo hacer macros dos veces
V568 Es extraño que el argumento del operador sizeof () sea 'bTextFrame? aProps: expresión de aShapeProps. wpscontext.cxx 134
oox::core::ContextHandlerRef WpsContext::onCreateContext(....) { .... OUString aProps[] = { .... }; OUString aShapeProps[] = { .... }; for (std::size_t i = 0; i < SAL_N_ELEMENTS(bTextFrame ? aProps : aShapeProps);
Desafortunadamente para muchos desarrolladores, los argumentos de macro no se comportan como argumentos de función. Ignorar este hecho a menudo conduce a errores. En los casos n. ° 1 y n. ° 2, se utiliza una construcción casi idéntica utilizando el operador ternario. Pero en el primer caso, una macro, en el segundo, una función. Sin embargo, esto es solo la parte superior del problema.
En el caso # 1, el analizador realmente detectó el siguiente código de error:
for (std::size_t i = 0; i < (sizeof (bTextFrame ? aProps : aShapeProps) / sizeof ((bTextFrame ? aProps : aShapeProps)[0])); ++i)
Este es nuestro bucle con la macro
SAL_N_ELEMENTS . El operador
sizeof no evalúa la expresión en el operador ternario. En este caso, la aritmética se realiza con el tamaño de los punteros, cuyo resultado son valores que están lejos del tamaño real de las matrices indicadas. El cálculo de valores incorrectos también se ve afectado por la profundidad de bits de la aplicación.
¡Pero luego resultó que hay 2 macros
SAL_N_ELEMENTS ! Es decir el preprocesador abrió la macro incorrecta, ¿cómo podría suceder esto? La definición de macro y los comentarios del desarrollador nos ayudarán a:
#ifndef SAL_N_ELEMENTS # if defined(__cplusplus) && ( defined(__GXX_EXPERIMENTAL_CXX0X__) || __cplusplus >= 201103L ) template <typename T, size_t S> char (&sal_n_array_size( T(&)[S] ))[S]; # define SAL_N_ELEMENTS(arr) (sizeof(sal_n_array_size(arr))) # else # define SAL_N_ELEMENTS(arr) (sizeof (arr) / sizeof ((arr)[0])) # endif #endif
Otra versión de la macro contiene una función de plantilla segura, pero algo salió mal:
- La macro segura no se incluyó en el código;
- Todavía es imposible usar otra macro, porque La instanciación exitosa de una función de plantilla se realiza solo si se transfieren matrices del mismo tamaño al operador ternario. Y en este caso, el uso de tal macro pierde su significado.
Errores tipográficos y copiar y pegar
V1013 Subexpresión sospechosa f1.Pitch == f2.CharSet en una secuencia de comparaciones similares. xmldlg_export.cxx 1251
inline bool equalFont( Style const & style1, Style const & style2 ) { awt::FontDescriptor const & f1 = style1._descr; awt::FontDescriptor const & f2 = style2._descr; return ( f1.Name == f2.Name && f1.Height == f2.Height && f1.Width == f2.Width && f1.StyleName == f2.StyleName && f1.Family == f2.Family && f1.CharSet == f2.CharSet &&
El error es un candidato digno para actualizar el artículo "El
mal vive en las funciones de comparación " si alguna vez decidimos actualizarlo o expandirlo. Creo que la probabilidad de encontrar tal error (pase
f2.Pitch ) solo es extremadamente pequeña. Que piensas
V501 Hay
subexpresiones idénticas 'mpTable [ocArrayColSep]! = MpTable [eOp]' a la izquierda y a la derecha del operador '&&'. formulacompiler.cxx 632
void FormulaCompiler::OpCodeMap::putOpCode(....) { .... case ocSep: bPutOp = true; bRemoveFromMap = (mpTable[eOp] != ";" && mpTable[ocArrayColSep] != mpTable[eOp] && mpTable[ocArrayColSep] != mpTable[eOp]); break; .... }
El resultado de una copia irreflexiva fue una pieza de código. Quizás la expresión condicional simplemente se duplica una vez más, pero aún en el código no hay lugar para tales ambigüedades.
V517 El uso del
patrón 'if (A) {...} else if (A) {...}' fue detectado. Hay una probabilidad de presencia de error lógico. Verifique las líneas: 781, 783. mysqlc_databasemetadata.cxx 781
Reference<XResultSet> SAL_CALL ODatabaseMetaData::getColumns(....) { .... bool bIsCharMax = !xRow->wasNull(); if (sDataType.equalsIgnoreAsciiCase("year")) nColumnSize = sColumnType.copy(6, 1).toInt32(); else if (sDataType.equalsIgnoreAsciiCase("date"))
Como resultado de copiar expresiones condicionales, se cometió un error en el código, debido al cual el valor
8 para la variable
nColumnSize nunca se establece.
V523 La declaración 'then' es equivalente a la declaración 'else'. svdpdf.hxx 146
Aquí las funciones
min () y
max () están confundidas. Seguramente, debido a este error tipográfico en la interfaz, algo tiene una escala extraña.
Ciclos extraños
V533 Es probable que se incremente una variable incorrecta dentro del operador 'for'. Considere revisar 'i'. javatypemaker.cxx 602
void printConstructors(....) { .... for (std::vector< unoidl::SingleInterfaceBasedServiceEntity::Constructor:: Parameter >::const_iterator j(i->parameters.begin()); j != i->parameters.end(); ++i) { o << ", "; printType(o, options, manager, j->type, false); if (j->rest) { o << "..."; } o << ' ' << codemaker::java::translateUnoToJavaIdentifier( u2b(j->name), "param"); } .... }
La expresión
++ i en el bucle parece muy sospechosa. Tal vez debería haber
++ j allí .
V756 El
contador 'nIndex2' no se usa dentro de un bucle anidado. Considere inspeccionar el uso del contador 'nIndex'. treex.cxx 34
SAL_IMPLEMENT_MAIN_WITH_ARGS(argc, argv) { OString sXHPRoot; for (int nIndex = 1; nIndex != argc; ++nIndex) { if (std::strcmp(argv[nIndex], "-r") == 0) { sXHPRoot = OString( argv[nIndex + 1] ); for( int nIndex2 = nIndex+3; nIndex2 < argc; nIndex2 = nIndex2 + 2 ) { argv[nIndex-3] = argv[nIndex-1]; argv[nIndex-2] = argv[nIndex]; } argc = argc - 2; break; } } common::HandledArgs aArgs; if( !common::handleArguments(argc, argv, aArgs) ) { WriteUsage(); return 1; } .... }
Hay algún tipo de error en el bucle
for interno . Porque la variable
nIndex no cambia; los mismos dos elementos de la matriz se sobrescriben en cada iteración. Lo más probable es que, en lugar de
nIndex , la variable
nIndex2 debería haberse usado en
todas partes .
V1008 Considere inspeccionar el operador 'para'. No se realizará más de una iteración del bucle. diagramhelper.cxx 292
void DiagramHelper::setStackMode( const Reference< XDiagram > & xDiagram, StackMode eStackMode ) { .... sal_Int32 nMax = aChartTypeList.getLength(); if( nMax >= 1 ) nMax = 1; for( sal_Int32 nT = 0; nT < nMax; ++nT ) { uno::Reference< XChartType > xChartType( aChartTypeList[nT] ); .... } .... }
El bucle
for está limitado intencionalmente a 1 iteración. No está claro por qué esto se hace de esta manera.
V612 Un 'retorno' incondicional dentro de un bucle. pormulti.cxx 891
SwTextAttr const* MergedAttrIterMulti::NextAttr(....) { .... SwpHints const*const pHints(m_pNode->GetpSwpHints()); if (pHints) { while (m_CurrentHint < pHints->Count()) { SwTextAttr const*const pHint(pHints->Get(m_CurrentHint)); ++m_CurrentHint; rpNode = m_pNode; return pHint; } } return nullptr; .... }
Un ejemplo de un bucle extraño más simple de una iteración, que es mejor reescribir en una declaración condicional.
Unos cuantos lugares más:
- V612 Un 'retorno' incondicional dentro de un bucle. txtfrm.cxx 144
- V612 Un 'retorno' incondicional dentro de un bucle. txtfrm.cxx 202
- V612 Un 'retorno' incondicional dentro de un bucle. txtfrm.cxx 279
Condiciones extrañas
V637 Se encontraron dos condiciones opuestas. La segunda condición es siempre falsa. Líneas de verificación: 281, 285. authfld.cxx 281
sal_uInt16 SwAuthorityFieldType::GetSequencePos(sal_IntPtr nHandle) { .... SwTOXSortTabBase* pOld = aSortArr[i].get(); if(*pOld == *pNew) {
El analizador encontró comparaciones conflictivas. Algo está claramente mal con este código.
El mismo código se ve en este lugar:
- V637 Se encontraron dos condiciones opuestas. La segunda condición es siempre falsa. Líneas de verificación: 1827, 1829. doctxm.cxx 1827
V590 Considere inspeccionar esta expresión. La expresión es excesiva o contiene un error de imprenta. fileurl.cxx 55
OUString convertToFileUrl(char const * filename, ....) { .... if ((filename[0] == '.') || (filename[0] != SEPARATOR)) { .... } .... }
El problema con el fragmento de código anterior es que la primera expresión condicional no afecta el resultado de toda la expresión.
Basado en tales errores, incluso escribí un artículo teórico: "
Expresiones lógicas en C / C ++. Cómo los profesionales están equivocados ".
V590 Considere inspeccionar esta expresión. La expresión es excesiva o contiene un error de imprenta. unoobj.cxx 1895
uno::Sequence< beans::PropertyState > SwUnoCursorHelper::GetPropertyStates(....) { .... if (((SW_PROPERTY_STATE_CALLER_SWX_TEXT_PORTION == eCaller) || (SW_PROPERTY_STATE_CALLER_SWX_TEXT_PORTION_TOLERANT == eCaller)) && pEntry->nWID < FN_UNO_RANGE_BEGIN && pEntry->nWID > FN_UNO_RANGE_END && pEntry->nWID < RES_CHRATR_BEGIN && pEntry->nWID > RES_TXTATR_END ) { pStates[i] = beans::PropertyState_DEFAULT_VALUE; } .... }
No entiendo de inmediato cuál es el problema de esta condición, por lo tanto, se escribió un fragmento de código expandido del archivo preprocesado:
if (((SW_PROPERTY_STATE_CALLER_SWX_TEXT_PORTION == eCaller) || (SW_PROPERTY_STATE_CALLER_SWX_TEXT_PORTION_TOLERANT == eCaller)) && pEntry->nWID < (20000 + 1600) && pEntry->nWID > ((20000 + 2400) + 199) && pEntry->nWID < 1 && pEntry->nWID > 63 ) { pStates[i] = beans::PropertyState_DEFAULT_VALUE; }
Sucedió que ni un solo número se incluye simultáneamente en los 4 rangos especificados en la condición por números. Los desarrolladores cometieron un error.
V590 Considere inspeccionar la expresión '* pData <= MAXLEVEL && * pData <= 9'. La expresión es excesiva o contiene un error de imprenta. ww8par2.cxx 756
const sal_uInt8 MAXLEVEL = 10; void SwWW8ImplReader::Read_ANLevelNo(....) { ....
Debido al hecho de que la primera condición usa una constante con un valor de
10 , la condición resultó ser redundante. Este código puede reescribirse de la siguiente manera:
if (*pData <= 9) { .... } else if( *pData == 10 || *pData == 11 ) { .... }
Pero tal vez hubo otro problema en el código presentado.
V757 Es posible que una variable incorrecta se compare con nullptr después de la conversión de tipo usando 'dynamic_cast'. Líneas de verificación: 2709, 2710. menu.cxx 2709
void PopupMenu::ClosePopup(Menu* pMenu) { MenuFloatingWindow* p = dynamic_cast<MenuFloatingWindow*>(ImplGetWindow()); PopupMenu *pPopup = dynamic_cast<PopupMenu*>(pMenu); if (p && pMenu) p->KillActivePopup(pPopup); }
Lo más probable es que la condición contenga un error. Fue necesario verificar los punteros
p y
pPopup .
V668 No tiene sentido probar el puntero 'm_pStream' contra nulo, ya que la memoria se asignó utilizando el operador 'nuevo'. La excepción se generará en caso de error de asignación de memoria. zipfile.cxx 408
ZipFile::ZipFile(const std::wstring &FileName) : m_pStream(nullptr), m_bShouldFree(true) { m_pStream = new FileStream(FileName.c_str()); if (m_pStream && !isZipStream(m_pStream)) { delete m_pStream; m_pStream = nullptr; } }
El analizador detectó una situación en la que el valor del puntero devuelto por el
nuevo operador se compara con cero. De acuerdo con el estándar del lenguaje C ++, si la asignación de memoria no es posible, el
nuevo operador lanzará una excepción
std :: bad_alloc . Solo se encontraron 45 de esos lugares en el proyecto LibreOffice, que es muy pequeño para tal volumen de código. Pero aún así, esto puede generar problemas para los usuarios. Los desarrolladores deben eliminar los controles innecesarios o crear objetos de esta manera:
m_pStream = new (std::nothrow) FileStream(FileName.c_str());
V728 Se puede simplificar una verificación excesiva. El '(A &&! B) || (! A && B) 'expresión es equivalente a la expresión' bool (A)! = Bool (B) '. toolbox2.cxx 1042
void ToolBox::SetItemImageMirrorMode( sal_uInt16 nItemId, bool bMirror ) { ImplToolItems::size_type nPos = GetItemPos( nItemId ); if ( nPos != ITEM_NOTFOUND ) { ImplToolItem* pItem = &mpData->m_aItems[nPos]; if ((pItem->mbMirrorMode && !bMirror) ||
Hace mucho tiempo, el diagnóstico
V728 se amplió a casos que, muy probablemente, no son erróneos, pero complican el código. Y en el código complejo, tarde o temprano aún se cometen errores.
Esta condición se simplifica a lo siguiente:
if (pItem->mbMirrorMode != bMirror) { .... }
Cerca de 60 estructuras similares se encuentran en el proyecto, algunas de ellas son muy, muy voluminosas. Es mejor que los autores del proyecto se familiaricen con el informe completo del analizador PVS-Studio.
Problemas de seguridad
V523 La declaración 'then' es equivalente a la declaración 'else'. docxattributeoutput.cxx 1571
void DocxAttributeOutput::DoWritePermissionTagEnd( const OUString & permission) { OUString permissionIdAndName; if (permission.startsWith("permission-for-group:", &permissionIdAndName)) { const sal_Int32 sparatorIndex = permissionIdAndName.indexOf(':'); const OUString permissionId = permissionIdAndName.copy(....); const OString rId = OUStringToOString(....).getStr(); m_pSerializer->singleElementNS(XML_w, XML_permEnd, FSNS(XML_w, XML_id), rId.getStr(), FSEND); } else { const sal_Int32 sparatorIndex = permissionIdAndName.indexOf(':'); const OUString permissionId = permissionIdAndName.copy(....); const OString rId = OUStringToOString(....).getStr(); m_pSerializer->singleElementNS(XML_w, XML_permEnd, FSNS(XML_w, XML_id), rId.getStr(), FSEND); } }
Aquí se copia un fragmento de código muy grande. Para una función que cambia ciertos derechos, el problema identificado parece muy sospechoso.
V1001 La variable 'DL' se asigna pero no se utiliza al final de la función. cipher.cxx 811
static void BF_updateECB( CipherContextBF *ctx, rtlCipherDirection direction, const sal_uInt8 *pData, sal_uInt8 *pBuffer, sal_Size nLength) { CipherKeyBF *key; sal_uInt32 DL, DR; key = &(ctx->m_key); if (direction == rtl_Cipher_DirectionEncode) { RTL_CIPHER_NTOHL64(pData, DL, DR, nLength); BF_encode(key, &DL, &DR); RTL_CIPHER_HTONL(DL, pBuffer); RTL_CIPHER_HTONL(DR, pBuffer); } else { RTL_CIPHER_NTOHL(pData, DL); RTL_CIPHER_NTOHL(pData, DR); BF_decode(key, &DL, &DR); RTL_CIPHER_HTONL64(DL, DR, pBuffer, nLength); } DL = DR = 0; }
Las variables
DL y
DR se anulan mediante una simple asignación al final de la función y ya no se usan. Para el compilador, esta es una excusa para eliminar comandos para la optimización.
Para limpiar adecuadamente las variables en las aplicaciones de Windows, puede volver a escribir el código de esta manera:
RtlSecureZeroMemory(&DL, sizeof(DL)); RtlSecureZeroMemory(&DR, sizeof(DR));
Pero para LibreOffice, se requiere una solución multiplataforma aquí.
Una advertencia similar de otra función:
- V1001 La variable 'DL' se asigna pero no se utiliza al final de la función. cipher.cxx 860
V764 Posible orden incorrecto de argumentos pasados a la función 'queryStream': 'rUri' y 'rPassword'. tdoc_storage.cxx 271
css::uno::Reference< css::io::XStream > queryStream( const css::uno::Reference< css::embed::XStorage > & xParentStorage, const OUString & rPassword, const OUString & rUri, StorageAccessMode eMode, bool bTruncate ); uno::Reference< io::XOutputStream > StorageElementFactory::createOutputStream( const OUString & rUri, const OUString & rPassword, bool bTruncate ) { .... uno::Reference< io::XStream > xStream = queryStream( xParentStorage, rUri, rPassword, READ_WRITE_CREATE, bTruncate ); .... }
Tenga en cuenta que en la función
queryStream , en la lista de parámetros,
rPassword primero y luego
rUri . Había un lugar en el código donde se mezclaban los argumentos correspondientes cuando se llamaba a esta función.
V794 El operador de asignación debe estar protegido del caso de 'this == & rToBeCopied'. hommatrixtemplate.hxx 121
ImplHomMatrixTemplate& operator=(....) {
Este caso trata más sobre la escritura segura de códigos. No hay verificación en la instrucción de copia para asignar un objeto a sí mismo. En total, el proyecto tiene alrededor de 30 implementaciones de este tipo del operador de copia.
Errores con SysAllocString
V649 Hay dos declaraciones 'if' con expresiones condicionales idénticas. La primera instrucción 'if' contiene la función return. Esto significa que la segunda declaración 'si' no tiene sentido. Líneas de verificación: 125, 137. acctable.cxx 137
STDMETHODIMP CAccTable::get_columnDescription(long column, BSTR * description) { SolarMutexGuard g; ENTER_PROTECTED_BLOCK
La función
SysAllocString () devuelve un puntero que puede ser
NULL . Algo extraño fue escrito por el autor de este código. El puntero a la memoria asignada no está marcado, lo que puede provocar problemas en el programa.
Advertencias similares de otras funciones:
- V649 Hay dos declaraciones 'if' con expresiones condicionales idénticas. La primera instrucción 'if' contiene la función return. Esto significa que la segunda declaración 'si' no tiene sentido. Líneas de verificación: 344, 356. acctable.cxx 356
- V649 Hay dos declaraciones 'if' con expresiones condicionales idénticas. La primera instrucción 'if' contiene la función return. Esto significa que la segunda declaración 'si' no tiene sentido. Líneas de verificación: 213, 219. trvlfrm.cxx 219
V530 Se requiere utilizar el valor de retorno de la función 'SysAllocString'. maccessible.cxx 1077
STDMETHODIMP CMAccessible::put_accValue(....) { .... if(varChild.lVal==CHILDID_SELF) { SysAllocString(m_pszValue); m_pszValue=SysAllocString(szValue); return S_OK; } .... }
No se utiliza el resultado de una de las llamadas a la función
SysAllocString () . Los desarrolladores deben prestar atención a este fragmento de código.
Misceláneo
V716 Conversión de tipo sospechosa en la declaración de retorno: devuelto HRESULT, pero la función en realidad devuelve BOOL. maccessible.cxx 2649
BOOL CMAccessible::get_IAccessibleFromXAccessible(....) { ENTER_PROTECTED_BLOCK
Una de las ramas de la ejecución de la función devuelve un valor cuyo tipo no coincide con el tipo del valor de retorno de la función.
El tipo HRESULT tiene un formato más complejo que el tipo BOOL y está diseñado para almacenar estados de operación. Por ejemplo, el valor de E_INVALIDARG es 0x80070057L . Será correcto escribir, por ejemplo, así: return FAILED(E_INVALIDARG);
Puede leer más sobre esto en la documentación de diagnóstico de V716.Algunos lugares más similares:- V716 Conversión de tipo sospechosa en la declaración de retorno: devuelto HRESULT, pero la función en realidad devuelve BOOL. inprocembobj.cxx 1299
- V716 Conversión de tipo sospechosa en la declaración de retorno: devuelto HRESULT, pero la función en realidad devuelve BOOL. maccessible.cxx 2660
V670 El miembro de clase no inicializado 'm_aMutex' se usa para inicializar el miembro 'm_aModifyListeners'. Recuerde que los miembros se inicializan en el orden de sus declaraciones dentro de una clase. fmgridif.cxx 1033 FmXGridPeer::FmXGridPeer(const Reference< XComponentContext >& _rxContext) :m_aModifyListeners(m_aMutex) ,m_aUpdateListeners(m_aMutex) ,m_aContainerListeners(m_aMutex) ,m_aSelectionListeners(m_aMutex) ,m_aGridControlListeners(m_aMutex) ,m_aMode( getDataModeIdentifier() ) ,m_nCursorListening(0) ,m_bInterceptingDispatch(false) ,m_xContext(_rxContext) {
De acuerdo con el estándar del lenguaje, el orden de inicialización de los miembros de la clase en el constructor ocurre en el orden de su declaración en la clase. En nuestro caso, el campo m_aMutex se inicializará después de participar en la inicialización de otros cinco campos de la clase.Así es como se ve el constructor de uno de los campos de clase: OInterfaceContainerHelper2( ::osl::Mutex & rMutex );
El objeto mutex se pasa por referencia. En este caso, pueden surgir varios problemas: desde acceder a la memoria no inicializada hasta la pérdida posterior de los cambios de objeto.El parámetro V763 'nNativeNumberMode' siempre se reescribe en el cuerpo de la función antes de usarse. calendar_jewish.cxx 286 OUString SAL_CALL Calendar_jewish::getDisplayString(...., sal_Int16 nNativeNumberMode ) {
Los argumentos de la función se sobrescriben por varias razones: advertencias del compilador de lucha, compatibilidad con versiones anteriores, muletas, etc. Sin embargo, cualquiera de estas soluciones no es buena y el código parece confuso.Debes revisar el código de este lugar y algunos más similares:- El parámetro V763 'bExtendedInfo' siempre se reescribe en el cuerpo de la función antes de usarse. graphicfilter2.cxx 442
- El parámetro V763 'nVerbID' siempre se reescribe en el cuerpo de la función antes de usarse. oleembed.cxx 841
- V763 Parameter 'pCursor' is always rewritten in function body before being used. edlingu.cxx 843
- V763 Parameter 'pOutput' is always rewritten in function body before being used. vnew.cxx 181
- V763 Parameter 'pOutput' is always rewritten in function body before being used. vnew.cxx 256
Conclusión
El deseo de verificar el código de LibreOffice apareció después de mi uso personal del producto. Por alguna razón, con algunos lanzamientos aleatorios, el texto desaparece de absolutamente todos los elementos del menú. Solo quedan iconos y rayas monótonas. El error es probablemente de alto nivel y, posiblemente, no se puede encontrar con la ayuda de herramientas de análisis estático. Sin embargo, el analizador encontró muchos problemas que no están relacionados con esto, y me alegrará si los desarrolladores de LibreOffice prestan atención a los analizadores de código estático y tratan de usarlos para mejorar la calidad y la confiabilidad del proyecto. Será útil para todos.Gracias por su atencion
¡Suscríbete a nuestros canales y mantente atento a las noticias del mundo de la programación!
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Svyatoslav Razmyslov. LibreOffice: la pesadilla del contador