O LibreOffice é um poderoso pacote de escritório gratuito para uso privado, educacional e comercial. Seus desenvolvedores estão criando um produto maravilhoso, que em muitas áreas é usado como uma alternativa ao Microsoft Office. A equipe do PVS-Studio está sempre interessada em examinar o código desses projetos conhecidos e tentar encontrar erros neles. Desta vez foi fácil de fazer. O projeto contém muitos erros que podem levar a problemas sérios. O artigo discutirá alguns defeitos interessantes encontrados no código.
1. Introdução
O LibreOffice é um projeto C ++ muito grande. Manter um projeto desse tamanho é uma tarefa difícil para a equipe de desenvolvimento. E, infelizmente, temos a impressão de que a qualidade do código do LibreOffice não presta atenção suficiente.
Por um lado, o projeto é simplesmente enorme, nem toda ferramenta de análise estática ou dinâmica pode lidar com a análise de 13k arquivos de código-fonte. Muitos arquivos estão envolvidos na construção do pacote de escritório, juntamente com bibliotecas de terceiros. O principal repositório do LibreOffice armazena cerca de 8k arquivos de código-fonte. Essa quantidade de código cria problemas não apenas para os desenvolvedores:
Por outro lado, o projeto possui muitos usuários e precisa encontrar e corrigir o máximo de erros possível. Todo erro pode prejudicar centenas e milhares de usuários. Portanto, o tamanho grande da base de código não deve se tornar uma desculpa para se recusar a usar determinadas ferramentas que podem detectar erros. Acho que o leitor já adivinhou que estamos falando de analisadores de código estático :).
Sim, o uso de analisadores estáticos não garante a ausência de erros no projeto. No entanto, ferramentas como o PVS-Studio podem encontrar um grande número de erros no estágio de desenvolvimento e, assim, reduzir a quantidade de trabalho associado à depuração e ao suporte ao projeto.
Vamos ver o que você pode achar interessante no código-fonte do LibreOffice se você
usar o analisador de código estático do
PVS-Studio . As possibilidades para executar o analisador são extensas: Windows, Linux, macOS. Para escrever esta revisão, foi utilizado o relatório PVS-Studio, criado durante a análise do projeto no Windows.
Alterações desde a última verificação em 2015
Em março de 2015, a primeira análise do LibreOffice ("
Verificação do projeto LibreOffice ") foi realizada usando o PVS-Studio. Desde então, a suíte de escritório evoluiu muito como um produto, mas por dentro também contém muitos erros. E alguns padrões de erro não mudaram desde então. Aqui, por exemplo, está um erro do primeiro artigo:
V656 As variáveis 'aVRP', 'aVPN' são inicializadas através da chamada para a mesma função. Provavelmente é um erro ou código não otimizado. Considere inspecionar a expressão 'rSceneCamera.GetVRP ()'. Verifique as linhas: 177, 178. viewcontactofe3dscene.cxx 178
void ViewContactOfE3dScene::createViewInformation3D(....) { .... const basegfx::B3DPoint aVRP(rSceneCamera.GetVRP()); const basegfx::B3DVector aVPN(rSceneCamera.GetVRP());
Este bug foi corrigido, mas aqui está o que foi encontrado na versão mais recente do código:
V656 As variáveis 'aSdvURL', 'aStrURL' são inicializadas por meio da chamada para a mesma função. Provavelmente é um erro ou código não otimizado. Considere inspecionar a expressão 'pThm-> GetSdvURL ()'. Verifique as linhas: 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 você deve ter notado, nomes sutis de funções compostas ainda são uma fonte de erros.
Outro exemplo interessante do código antigo:
V656 As variáveis 'nDragW', 'nDragH' são inicializadas por meio da chamada para a mesma função. Provavelmente é um erro ou código não otimizado. Considere inspecionar a expressão 'rMSettings.GetStartDragWidth ()'. Verifique as linhas: 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 pedaço de código realmente continha um bug que agora está corrigido. Mas os erros no código não estão diminuindo ... Uma situação semelhante agora foi identificada:
V656 As variáveis 'defaultZoomX', 'defaultZoomY' são inicializadas por meio da chamada para a mesma função. Provavelmente é um erro ou código não otimizado. Considere inspecionar a expressão 'pViewData-> GetZoomX ()'. Verifique as linhas: 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();
Os erros são introduzidos no código literalmente por analogia.
Não se deixe enganar
V765 Uma expressão de atribuição composta 'x - = x - ...' é suspeita. Considere inspecioná-lo para um possível erro. swdtflvr.cxx 3509
bool SwTransferable::PrivateDrop(...) { .... if ( rSrcSh.IsSelFrameMode() ) {
Aqui está um "Hack" tão interessante foi encontrado usando o diagnóstico
V765 . Se você simplificar uma linha de código com um comentário, poderá obter um resultado inesperado:
1º passo:
aSttPt = aSttPt - (aSttPt - rSrcSh.GetObjRect().Pos());
2º passo:
aSttPt = aSttPt - aSttPt + rSrcSh.GetObjRect().Pos();
3º passo
aSttPt = rSrcSh.GetObjRect().Pos();
E então o que é hack?
Outro exemplo sobre este tópico:
V567 A modificação da variável 'nCount' não ocorre em relação a outra operação na mesma variável. Isso pode levar a um comportamento indefinido. stgio.cxx 214
FatError EasyFat::Mark(....) { if( nCount > 0 ) { --nCount /= GetPageSize(); nCount++; } .... }
A execução de código nessas situações pode depender do compilador e do padrão de linguagem. Por que não reescrever esse trecho de código de uma maneira mais simples, clara e confiável?
Como não usar matrizes e vetores
Por alguma razão, alguém cometeu muitos erros semelhantes ao trabalhar com matrizes e vetores. Vejamos esses exemplos.
A saturação da matriz V557 é possível. O índice 'nPageNum' está apontando além do limite da matriz. pptx-epptooxml.cxx 1168
void PowerPointExport::ImplWriteNotes(sal_uInt32 nPageNum) { ....
O último índice válido deve ser um valor igual a
size () - 1 . Mas neste fragmento de código, uma situação foi permitida quando o índice
nPageNum pode ter o valor
mpSlidesFSArray.size () , pelo qual existe uma matriz fora dos limites e trabalha com um elemento que consiste em "lixo".
A saturação da matriz V557 é possível. O índice 'mnSelectedMenu' está apontando além do limite da matriz. checklistmenu.cxx 826
void ScMenuFloatingWindow::ensureSubMenuNotVisible() { if (mnSelectedMenu <= maMenuItems.size() && maMenuItems[mnSelectedMenu].mpSubMenuWin && maMenuItems[mnSelectedMenu].mpSubMenuWin->IsVisible()) { maMenuItems[mnSelectedMenu].mpSubMenuWin->ensureSubMenuNotVisible(); } EndPopupMode(); }
Curiosamente, nesse fragmento de código, eles escreveram uma verificação de índice mais claramente, mas ao mesmo tempo cometeram o mesmo erro.
A saturação da matriz V557 é possível. O índice 'nXFIndex' está apontando além do limite da matriz. xestyle.cxx 2613
sal_Int32 XclExpXFBuffer::GetXmlStyleIndex( sal_uInt32 nXFIndex ) const { OSL_ENSURE( nXFIndex < maStyleIndexes.size(), "...." ); if( nXFIndex > maStyleIndexes.size() ) return 0;
E esse erro é duplamente interessante! Na macro de depuração, eles escreveram a verificação correta do índice e, em outro lugar, novamente cometeram um erro, permitindo que saíssem da matriz.
Agora considere um tipo diferente de erro que não está relacionado aos índices.
V554 Uso incorreto de shared_ptr. A memória alocada com 'new []' será limpa usando 'delete'. dx_vcltools.cxx 158
struct RawRGBABitmap { sal_Int32 mnWidth; sal_Int32 mnHeight; std::shared_ptr< sal_uInt8 > mpBitmapData; }; RawRGBABitmap bitmapFromVCLBitmapEx( const ::BitmapEx& rBmpEx ) { ....
Este pedaço de código contém um erro que leva ao comportamento indefinido do programa. O fato é que a memória é alocada e liberada de maneiras diferentes. Para liberar memória corretamente, você tinha que declarar um campo de classe como este:
std::shared_ptr< sal_uInt8[] > mpBitmapData;
Como fazer macros duas vezes
V568 É estranho que o argumento do operador sizeof () seja o 'bTextFrame? aProps: expressão 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);
Infelizmente para muitos desenvolvedores, os argumentos de macro não se comportam como argumentos de função. Ignorar esse fato geralmente leva a erros. Nos casos 1 e 2, uma construção quase idêntica é usada usando o operador ternário. Mas no primeiro caso - uma macro, no segundo - uma função. No entanto, este é apenas o topo do problema.
No caso 1, o analisador realmente detectou o seguinte código de erro:
for (std::size_t i = 0; i < (sizeof (bTextFrame ? aProps : aShapeProps) / sizeof ((bTextFrame ? aProps : aShapeProps)[0])); ++i)
Este é o nosso loop com a macro
SAL_N_ELEMENTS . O operador
sizeof não avalia a expressão no operador ternário. Nesse caso, a aritmética é realizada com o tamanho dos ponteiros, cujos resultados são valores que estão distantes do tamanho real das matrizes indicadas. O cálculo de valores incorretos também é afetado pela profundidade de bits do aplicativo.
Mas, em seguida, verificou-se que existem 2 macros
SAL_N_ELEMENTS ! I.e. o pré-processador abriu a macro errada, como isso pôde acontecer? A definição da macro e os comentários do desenvolvedor nos ajudarão 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
Outra versão da macro contém uma função de modelo seguro, mas algo deu errado:
- A macro segura não foi incluída no código;
- Ainda é impossível usar outra macro, porque a instanciação bem-sucedida de uma função de modelo é executada apenas se matrizes do mesmo tamanho forem transferidas para o operador ternário. E, neste caso, o uso de uma macro perde seu significado.
Erros de digitação e copiar e colar
V1013 Subexpressão suspeita f1.Pitch == f2.CharSet em uma sequência de comparações semelhantes. 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 &&
O erro é um candidato digno para atualizar o artigo “O
mal vive em funções de comparação ” se decidirmos atualizá-lo ou expandi-lo. Eu acho que a probabilidade de encontrar esse erro (passe
f2.Pitch ) sozinha é extremamente pequena. O que você acha?
V501 Existem
subexpressões idênticas 'mpTable [ocArrayColSep]! = MpTable [eOp]' à esquerda e à direita do operador '&&'. formulacompiler.cxx 632
void FormulaCompiler::OpCodeMap::putOpCode(....) { .... case ocSep: bPutOp = true; bRemoveFromMap = (mpTable[eOp] != ";" && mpTable[ocArrayColSep] != mpTable[eOp] && mpTable[ocArrayColSep] != mpTable[eOp]); break; .... }
O resultado de uma cópia impensada foi um pedaço de código. Talvez a expressão condicional seja simplesmente duplicada mais uma vez, mas ainda no código não há lugar para essas ambiguidades.
V517 O uso do
padrão 'if (A) {...} else if (A) {...}' foi detectado. Há uma probabilidade de presença de erro lógico. Verifique as linhas: 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 da cópia de expressões condicionais, ocorreu um erro no código, devido ao qual o valor
8 da variável
nColumnSize nunca
é definido.
V523 A
instrução 'then' é equivalente à instrução 'else'. svdpdf.hxx 146
Aqui as funções
min () e
max () são confusas. Certamente, devido a esse erro de digitação na interface, algo é estranhamente dimensionado.
Ciclos estranhos
V533 É provável que uma variável incorreta esteja sendo incrementada dentro do 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"); } .... }
A expressão
++ i no loop parece muito suspeita. Talvez deva haver
++ j lá .
V756 O contador 'nIndex2' não é usado dentro de um loop aninhado. Considere inspecionar o uso do 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; } .... }
Há algum tipo de erro no loop
for interno . Porque a variável
nIndex não muda; os mesmos dois elementos da matriz são substituídos a cada iteração. Provavelmente, em vez de
nIndex , a variável
nIndex2 deveria ter sido usada em
qualquer lugar .
V1008 Considere inspecionar o operador 'for'. Não será executada mais de uma iteração do loop. 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] ); .... } .... }
O loop for é intencionalmente limitado a 1 iteração. Não está claro por que isso é feito dessa maneira.
V612 Um 'retorno' incondicional dentro de um loop. 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; .... }
Um exemplo de um loop estranho mais simples de uma iteração, que é melhor reescrever em uma declaração condicional.
Mais alguns desses lugares:
- V612 Um 'retorno' incondicional dentro de um loop. txtfrm.cxx 144
- V612 Um 'retorno' incondicional dentro de um loop. txtfrm.cxx 202
- V612 Um 'retorno' incondicional dentro de um loop. txtfrm.cxx 279
Condições estranhas
V637 Foram
encontradas duas condições opostas. A segunda condição é sempre falsa. Verifique as linhas: 281, 285. authfld.cxx 281
sal_uInt16 SwAuthorityFieldType::GetSequencePos(sal_IntPtr nHandle) { .... SwTOXSortTabBase* pOld = aSortArr[i].get(); if(*pOld == *pNew) {
O analisador encontrou comparações conflitantes. Algo está claramente errado com esse trecho de código.
O mesmo código é visto neste local:
- V637 Foram encontradas duas condições opostas. A segunda condição é sempre falsa. Verifique as linhas: 1827, 1829. doctxm.cxx 1827
V590 Considere inspecionar esta expressão. A expressão é excessiva ou contém uma impressão incorreta. fileurl.cxx 55
OUString convertToFileUrl(char const * filename, ....) { .... if ((filename[0] == '.') || (filename[0] != SEPARATOR)) { .... } .... }
O problema com o fragmento de código acima é que a primeira expressão condicional não afeta o resultado de toda a expressão.
Com base nesses erros, escrevi até um artigo teórico: "
Expressões lógicas em C / C ++. Como os profissionais estão errados ".
V590 Considere inspecionar esta expressão. A expressão é excessiva ou contém uma impressão incorreta. 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; } .... }
Como não entendo imediatamente qual é o problema dessa condição, um fragmento de código expandido foi gravado no arquivo pré-processado:
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; }
Aconteceu que nem um único número é incluído simultaneamente nos 4 intervalos especificados na condição por números. Os desenvolvedores cometeram um erro.
V590 Considere inspecionar a expressão '* pData <= MAXLEVEL && * pData <= 9'. A expressão é excessiva ou contém uma impressão incorreta. ww8par2.cxx 756
const sal_uInt8 MAXLEVEL = 10; void SwWW8ImplReader::Read_ANLevelNo(....) { ....
Devido ao fato de a primeira condição usar uma constante com um valor de
10 , a condição acabou sendo redundante. Este trecho de código pode ser reescrito da seguinte maneira:
if (*pData <= 9) { .... } else if( *pData == 10 || *pData == 11 ) { .... }
Mas talvez houvesse outro problema no código apresentado.
V757 É possível que uma variável incorreta seja comparada com nullptr após a conversão do tipo usando 'dynamic_cast'. Verifique as linhas: 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); }
Provavelmente, a condição contém um erro. Foi necessário verificar os ponteiros
pepPopup .
V668 Não há sentido em testar o ponteiro 'm_pStream' contra nulo, pois a memória foi alocada usando o operador 'novo'. A exceção será gerada no caso de erro de alocação de memória. 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; } }
O analisador detectou uma situação em que o valor do ponteiro retornado pelo
novo operador é comparado a zero. De acordo com o padrão da linguagem C ++, se a alocação de memória não for possível, o
novo operador lançará uma exceção
std :: bad_alloc . Apenas 45 desses locais foram encontrados no projeto LibreOffice, que é muito pequeno para esse volume de código. Mas, ainda assim, isso pode levar a problemas para os usuários. Os desenvolvedores devem remover verificações desnecessárias ou criar objetos da seguinte maneira:
m_pStream = new (std::nothrow) FileStream(FileName.c_str());
V728 Uma verificação excessiva pode ser simplificada. O '(A &&! B) || (! A && B) 'expressão é equivalente à expressão' 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) ||
Há muito tempo, o diagnóstico da
V728 foi expandido para casos que, provavelmente, não são errôneos, mas complicam o código. E em código complexo, erros mais cedo ou mais tarde ainda são cometidos.
Essa condição é simplificada para o seguinte:
if (pItem->mbMirrorMode != bMirror) { .... }
Cerca de 60 estruturas semelhantes são encontradas no projeto, algumas delas são muito, muito volumosas. Os autores do projeto são melhores para se familiarizar com o relatório completo do analisador PVS-Studio.
Questões de segurança
V523 A
instrução 'then' é equivalente à instrução '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); } }
Um pedaço muito grande de código é copiado aqui. Para uma função que altera certos direitos, o problema identificado parece muito suspeito.
V1001 A variável 'DL' é atribuída, mas não é usada no final da função. 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; }
As variáveis
DL e
DR são anuladas por uma atribuição simples no final da função e não são mais usadas. Para o compilador, essa é uma desculpa para remover comandos para otimização.
Para limpar adequadamente as variáveis nos aplicativos Windows, você pode reescrever o código da seguinte maneira:
RtlSecureZeroMemory(&DL, sizeof(DL)); RtlSecureZeroMemory(&DR, sizeof(DR));
Mas para o LibreOffice, uma solução de plataforma cruzada é necessária aqui.
Um aviso semelhante de outra função:
- V1001 A variável 'DL' é atribuída, mas não é usada no final da função. cipher.cxx 860
V764 Possível ordem incorreta de argumentos transmitida para a função 'queryStream': 'rUri' e '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 ); .... }
Observe que, na função
queryStream , na lista de parâmetros,
rPassword primeiro e depois
rUri . Havia um lugar no código em que os argumentos correspondentes eram misturados quando essa função foi chamada.
V794 O operador de atribuição deve ser protegido do caso de 'this == & rToBeCopied'. hommatrixtemplate.hxx 121
ImplHomMatrixTemplate& operator=(....) {
Este caso é mais sobre escrita segura de código. Não há verificação na declaração de cópia para atribuir um objeto a si próprio. No total, o projeto possui cerca de 30 implementações desse tipo de operador de cópia.
Erros com SysAllocString
V649 Existem duas instruções 'if' com expressões condicionais idênticas. A primeira instrução 'if' contém retorno de função. Isso significa que a segunda declaração 'if' não faz sentido. Verifique as linhas: 125, 137. acctable.cxx 137
STDMETHODIMP CAccTable::get_columnDescription(long column, BSTR * description) { SolarMutexGuard g; ENTER_PROTECTED_BLOCK
A função
SysAllocString () retorna um ponteiro que pode ser
NULL . Algo estranho foi escrito pelo autor deste código. O ponteiro para a memória alocada não está marcado, o que pode causar problemas no programa.
Avisos semelhantes de outras funções:
- V649 Existem duas instruções 'if' com expressões condicionais idênticas. A primeira instrução 'if' contém retorno de função. Isso significa que a segunda declaração 'if' não faz sentido. Verifique as linhas: 344, 356. acctable.cxx 356
- V649 Existem duas instruções 'if' com expressões condicionais idênticas. A primeira instrução 'if' contém retorno de função. Isso significa que a segunda declaração 'if' não faz sentido. Verifique as linhas: 213, 219. trvlfrm.cxx 219
V530 O valor de retorno da função 'SysAllocString' deve ser utilizado. maccessible.cxx 1077
STDMETHODIMP CMAccessible::put_accValue(....) { .... if(varChild.lVal==CHILDID_SELF) { SysAllocString(m_pszValue); m_pszValue=SysAllocString(szValue); return S_OK; } .... }
O resultado de uma das chamadas para a função
SysAllocString () não é usado. Os desenvolvedores devem prestar atenção a esse pedaço de código.
Diversos
V716 Conversão de tipo suspeita na instrução de retorno: retornou HRESULT, mas a função realmente retorna BOOL. maccessible.cxx 2649
BOOL CMAccessible::get_IAccessibleFromXAccessible(....) { ENTER_PROTECTED_BLOCK
Um dos ramos de execução de uma função retorna um valor cujo tipo não corresponde ao tipo do valor de retorno da função.
O tipo HRESULT possui um formato mais complexo que o tipo BOOL e foi projetado para armazenar status de operação. Por exemplo, o valor de E_INVALIDARG é 0x80070057L . Será correto escrever, por exemplo, assim: return FAILED(E_INVALIDARG);
Você pode ler mais sobre isso na documentação de diagnóstico do V716.Mais alguns lugares semelhantes:- V716 Conversão de tipo suspeita na instrução de retorno: retornou HRESULT, mas a função realmente retorna BOOL. inprocembobj.cxx 1299
- V716 Conversão de tipo suspeita na instrução de retorno: retornou HRESULT, mas a função realmente retorna BOOL. maccessible.cxx 2660
V670 O membro da classe não inicializado 'm_aMutex' é usado para inicializar o membro 'm_aModifyListeners'. Lembre-se de que os membros são inicializados na ordem de suas declarações dentro de uma classe. 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 acordo com o padrão de idioma, a ordem de inicialização dos membros da classe no construtor ocorre na ordem em que são declarados na classe. No nosso caso, o campo m_aMutex será inicializado após a participação na inicialização de outros cinco campos da classe.Aqui está a aparência do construtor de um dos campos de classe: OInterfaceContainerHelper2( ::osl::Mutex & rMutex );
O objeto mutex é passado por referência. Nesse caso, vários problemas podem surgir: do acesso à memória não inicializada e à subsequente perda de alterações no objeto.O parâmetro V763 'nNativeNumberMode' sempre é reescrito no corpo da função antes de ser usado. calendar_jewish.cxx 286 OUString SAL_CALL Calendar_jewish::getDisplayString(...., sal_Int16 nNativeNumberMode ) {
Os argumentos de função são substituídos por vários motivos: combatendo avisos do compilador, compatibilidade com versões anteriores, muleta, etc. No entanto, qualquer uma dessas soluções não é boa e o código parece confuso.Você deve revisar o código deste local e mais alguns similares:- V763 O parâmetro 'bExtendedInfo' é sempre reescrito no corpo da função antes de ser usado. graphicfilter2.cxx 442
- V763 O parâmetro 'nVerbID' é sempre reescrito no corpo da função antes de ser usado. 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
Conclusão
O desejo de verificar o código do LibreOffice apareceu após meu uso pessoal do produto. Por alguma razão, com algumas ativações aleatórias, o texto desaparece de absolutamente todos os itens de menu. Apenas ícones e faixas monótonas permanecem. O erro provavelmente é de alto nível e, possivelmente, não pode ser encontrado com a ajuda de ferramentas de análise estática. No entanto, o analisador encontrou muitos problemas não relacionados a isso, e ficarei feliz se os desenvolvedores do LibreOffice prestarem atenção nos analisadores de código estático e tentarem usá-los para melhorar a qualidade e a confiabilidade do projeto. Será útil para todos.Obrigado pela atenção.
Assine nossos canais e fique atento às novidades do mundo da programação!
Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Svyatoslav Razmyslov. LibreOffice: Pesadelo do contador