LibreOffice est une puissante suite bureautique gratuite pour un usage privé, éducatif et commercial. Ses développeurs font un produit merveilleux, qui dans de nombreux domaines est utilisé comme une alternative à Microsoft Office. L'équipe de PVS-Studio est toujours intéressée à regarder le code de ces projets bien connus et à essayer d'y trouver des erreurs. Cette fois, c'était facile à faire. Le projet contient de nombreuses erreurs pouvant entraîner de graves problèmes. L'article discutera de quelques défauts intéressants trouvés dans le code.
Présentation
LibreOffice est un très grand projet C ++. La maintenance d'un projet de cette taille est une tâche difficile pour l'équipe de développement. Et, malheureusement, on a l'impression que la qualité du code LibreOffice ne fait pas assez attention.
D'une part, le projet est tout simplement énorme, tous les outils d'analyse statique ou dynamique ne peuvent pas gérer l'analyse des fichiers de code source 13k. Tant de fichiers sont impliqués dans la construction de la suite bureautique avec des bibliothèques tierces. Le référentiel principal de LibreOffice stocke environ 8 000 fichiers de code source. Cette quantité de code crée des problèmes non seulement pour les développeurs:
D'un autre côté, le projet a de nombreux utilisateurs et doit trouver et corriger autant d'erreurs que possible. Chaque erreur peut blesser des centaines et des milliers d'utilisateurs. Par conséquent, la grande taille de la base de code ne doit pas devenir une excuse pour refuser d'utiliser certains outils qui peuvent détecter des erreurs. Je pense que le lecteur a déjà deviné que nous parlons d'analyseurs de code statique :).
Oui, l'utilisation d'analyseurs statiques ne garantit pas l'absence d'erreurs dans le projet. Cependant, des outils tels que PVS-Studio sont capables de trouver un grand nombre d'erreurs au stade du développement et ainsi de réduire la quantité de travail associée au débogage et au support de projet.
Voyons ce que vous pouvez trouver d'intéressant dans le code source de LibreOffice si vous prenez l'analyseur de code statique
PVS-Studio . Les possibilités d'exécution de l'analyseur sont nombreuses: Windows, Linux, macOS. Pour écrire cette revue, le rapport PVS-Studio a été utilisé, créé lors de l'analyse du projet sous Windows.
Changements depuis le dernier contrôle en 2015
En mars 2015, la première analyse de LibreOffice ("
Vérification du projet LibreOffice ") a été réalisée à l'aide de PVS-Studio. Depuis lors, la suite bureautique a beaucoup évolué en tant que produit, mais elle contient également de nombreuses erreurs. Et certains modèles d'erreur n'ont pas changé du tout depuis lors. Voici, par exemple, une erreur du premier article:
V656 Les variables 'aVRP', 'aVPN' sont initialisées via l'appel à la même fonction. Il s'agit probablement d'une erreur ou d'un code non optimisé. Pensez à inspecter l'expression «rSceneCamera.GetVRP ()». Vérifiez les lignes: 177, 178. viewcontactofe3dscene.cxx 178
void ViewContactOfE3dScene::createViewInformation3D(....) { .... const basegfx::B3DPoint aVRP(rSceneCamera.GetVRP()); const basegfx::B3DVector aVPN(rSceneCamera.GetVRP());
Ce bug a été corrigé, mais voici ce qui a été trouvé dans la dernière version du code:
V656 Les variables 'aSdvURL', 'aStrURL' sont initialisées via l'appel à la même fonction. Il s'agit probablement d'une erreur ou d'un code non optimisé. Pensez à inspecter l'expression «pThm-> GetSdvURL ()». Vérifier les lignes: 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() );
Comme vous l'avez peut-être remarqué, les noms de fonction composés subtils sont toujours une source d'erreurs.
Un autre exemple intéressant de l'ancien code:
V656 Les variables 'nDragW', 'nDragH' sont initialisées via l'appel à la même fonction. Il s'agit probablement d'une erreur ou d'un code non optimisé. Pensez à inspecter l'expression «rMSettings.GetStartDragWidth ()». Vérifier les lignes: 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(); .... }
Ce morceau de code contenait vraiment un bug qui est maintenant corrigé. Mais les erreurs dans le code ne diminuent pas ... Une situation similaire a maintenant été identifiée:
V656 Les variables 'defaultZoomX', 'defaultZoomY' sont initialisées via l'appel à la même fonction. Il s'agit probablement d'une erreur ou d'un code non optimisé. Pensez à inspecter l'expression «pViewData-> GetZoomX ()». Vérifiez les lignes: 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();
Les erreurs sont introduites dans le code littéralement par analogie.
Ne vous laissez pas berner
V765 Une expression d'affectation composée 'x - = x - ...' est suspecte. Pensez à l'inspecter pour une éventuelle erreur. swdtflvr.cxx 3509
bool SwTransferable::PrivateDrop(...) { .... if ( rSrcSh.IsSelFrameMode() ) {
Voici un "Hack" si intéressant qui a été trouvé en utilisant le diagnostic
V765 . Si vous simplifiez une ligne de code avec un commentaire, vous pouvez obtenir un résultat inattendu:
1ère étape:
aSttPt = aSttPt - (aSttPt - rSrcSh.GetObjRect().Pos());
2ème étape:
aSttPt = aSttPt - aSttPt + rSrcSh.GetObjRect().Pos();
3e étape
aSttPt = rSrcSh.GetObjRect().Pos();
Et puis qu'est-ce que le hack?
Un autre exemple sur ce sujet:
V567 La modification de la variable 'nCount' n'est pas séquencée par rapport à une autre opération sur la même variable. Cela peut conduire à un comportement indéfini. stgio.cxx 214
FatError EasyFat::Mark(....) { if( nCount > 0 ) { --nCount /= GetPageSize(); nCount++; } .... }
L'exécution de code dans de telles situations peut dépendre du compilateur et de la norme de langage. Pourquoi ne pas réécrire cet extrait de code d'une manière plus simple, plus claire et plus fiable?
Comment ne pas utiliser les tableaux et les vecteurs
Pour une raison quelconque, quelqu'un a fait beaucoup d'erreurs similaires en travaillant avec des tableaux et des vecteurs. Regardons ces exemples.
V557 Le dépassement de matrice est possible. L'index 'nPageNum' pointe au-delà de la limite du tableau. pptx-epptooxml.cxx 1168
void PowerPointExport::ImplWriteNotes(sal_uInt32 nPageNum) { ....
Le dernier index valide doit être une valeur égale à
size () - 1 . Mais dans ce fragment de code, une situation a été autorisée lorsque l'index
nPageNum peut avoir la valeur
mpSlidesFSArray.size () , à cause de laquelle il existe un tableau hors limites et fonctionne avec un élément composé de "garbage".
V557 Le dépassement de matrice est possible. L'index 'mnSelectedMenu' pointe au-delà de la limite du tableau. checklistmenu.cxx 826
void ScMenuFloatingWindow::ensureSubMenuNotVisible() { if (mnSelectedMenu <= maMenuItems.size() && maMenuItems[mnSelectedMenu].mpSubMenuWin && maMenuItems[mnSelectedMenu].mpSubMenuWin->IsVisible()) { maMenuItems[mnSelectedMenu].mpSubMenuWin->ensureSubMenuNotVisible(); } EndPopupMode(); }
Fait intéressant, dans ce fragment de code, ils ont écrit une vérification d'index plus clairement, mais en même temps, ils ont fait la même erreur.
V557 Le dépassement de matrice est possible. L'index 'nXFIndex' pointe au-delà de la limite du tableau. xestyle.cxx 2613
sal_Int32 XclExpXFBuffer::GetXmlStyleIndex( sal_uInt32 nXFIndex ) const { OSL_ENSURE( nXFIndex < maStyleIndexes.size(), "...." ); if( nXFIndex > maStyleIndexes.size() ) return 0;
Et cette erreur est doublement intéressante! Dans la macro de débogage, ils ont écrit la vérification d'index correcte et, à un autre endroit, ils ont à nouveau fait une erreur, leur permettant de sortir du tableau.
Considérons maintenant un autre type d'erreur qui n'est pas lié aux index.
V554 Utilisation incorrecte de shared_ptr. La mémoire allouée avec «nouveau []» sera nettoyée à l'aide de «supprimer». dx_vcltools.cxx 158
struct RawRGBABitmap { sal_Int32 mnWidth; sal_Int32 mnHeight; std::shared_ptr< sal_uInt8 > mpBitmapData; }; RawRGBABitmap bitmapFromVCLBitmapEx( const ::BitmapEx& rBmpEx ) { ....
Ce morceau de code contient une erreur conduisant à un comportement de programme non défini. Le fait est que la mémoire est allouée et libérée de différentes manières. Pour libérer correctement la mémoire, vous devez déclarer un champ de classe comme celui-ci:
std::shared_ptr< sal_uInt8[] > mpBitmapData;
Comment créer des macros deux fois
V568 Il est étrange que l'argument de l'opérateur sizeof () soit le 'bTextFrame? aProps: expression 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);
Malheureusement pour de nombreux développeurs, les arguments de macro ne se comportent pas comme des arguments de fonction. Ignorer ce fait conduit souvent à des erreurs. Dans les cas # 1 et # 2, une construction presque identique est utilisée en utilisant l'opérateur ternaire. Mais dans le premier cas - une macro, dans le second - une fonction. Cependant, ce n'est que le sommet du problème.
Dans le cas # 1, l'analyseur a effectivement détecté le code d'erreur suivant:
for (std::size_t i = 0; i < (sizeof (bTextFrame ? aProps : aShapeProps) / sizeof ((bTextFrame ? aProps : aShapeProps)[0])); ++i)
C'est notre boucle avec la macro
SAL_N_ELEMENTS . L'opérateur
sizeof n'évalue pas l'expression dans l'opérateur ternaire. Dans ce cas, l'arithmétique est effectuée avec la taille des pointeurs, dont le résultat est des valeurs qui sont loin de la taille réelle des tableaux indiqués. Le calcul des valeurs incorrectes est en outre affecté par la profondeur de bits de l'application.
Mais il s'est avéré qu'il y avait 2 macros
SAL_N_ELEMENTS ! C'est-à-dire le préprocesseur a ouvert la mauvaise macro, comment est-ce possible? La définition de la macro et les commentaires des développeurs nous aideront à:
#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
Une autre version de la macro contient une fonction de modèle sûre, mais quelque chose s'est mal passé:
- La macro sûre n'était pas incluse dans le code;
- Il est toujours impossible d'utiliser une autre macro, car l'instanciation réussie d'une fonction de modèle n'est effectuée que si des tableaux de même taille sont transférés à l'opérateur ternaire. Et dans ce cas, l'utilisation d'une telle macro perd son sens.
Typos et copier-coller
V1013 Sous- expression suspecte f1.Pitch == f2.CharSet dans une séquence de comparaisons similaires. 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 &&
L'erreur est un bon candidat pour la mise à jour de l'article «
Evil lives dans les fonctions de comparaison » si jamais nous décidons de le mettre à jour ou de l'étendre. Je pense que la probabilité de trouver une telle erreur (passer
f2.Pitch ) seule est extrêmement faible. Qu'en penses-tu?
V501 Il existe des sous-expressions identiques «mpTable [ocArrayColSep]! = MpTable [eOp]» à gauche et à droite de l'opérateur «&&». formulacompiler.cxx 632
void FormulaCompiler::OpCodeMap::putOpCode(....) { .... case ocSep: bPutOp = true; bRemoveFromMap = (mpTable[eOp] != ";" && mpTable[ocArrayColSep] != mpTable[eOp] && mpTable[ocArrayColSep] != mpTable[eOp]); break; .... }
Le résultat d'une copie irréfléchie était un tel morceau de code. Peut-être que l'expression conditionnelle est simplement dupliquée encore une fois, mais toujours dans le code il n'y a pas de place pour de telles ambiguïtés.
V517 L'utilisation du
modèle 'if (A) {...} else if (A) {...}' a été détectée. Il y a une probabilité de présence d'erreur logique. Vérifiez les lignes: 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"))
À la suite de la copie d'expressions conditionnelles, une erreur a été commise dans le code, à cause de laquelle la valeur
8 pour la variable
nColumnSize n'est jamais définie.
V523 L'
instruction 'then' est équivalente à l'instruction 'else'. svdpdf.hxx 146
Ici, les fonctions
min () et
max () sont confondues. Sûrement, à cause de cette faute de frappe dans l'interface, quelque chose est étrangement mis à l'échelle.
Cycles étranges
V533 Il est probable qu'une mauvaise variable soit incrémentée à l'intérieur de l'opérateur 'for'. Pensez à revoir «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"); } .... }
L'expression
++ i dans la boucle semble très suspecte. Il devrait peut-être y avoir
++ j .
V756 Le
compteur 'nIndex2' n'est pas utilisé dans une boucle imbriquée. Envisagez d'inspecter l'utilisation du compteur «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; } .... }
Il y a une sorte d'erreur dans la boucle
for interne . Parce que la variable
nIndex ne change pas; les deux mêmes éléments du tableau sont remplacés à chaque itération. Très probablement, au lieu de
nIndex , la variable
nIndex2 aurait dû être utilisée
partout .
V1008 Envisagez d'inspecter l'opérateur «pour». Pas plus d'une itération de la boucle sera effectuée. 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] ); .... } .... }
La boucle
for est intentionnellement limitée à 1 itération. On ne sait pas pourquoi cela se fait de cette façon.
V612 Un «retour» inconditionnel dans une boucle. 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 exemple d'une boucle bizarre plus simple à partir d'une itération, qui est préférable de réécrire dans une instruction conditionnelle.
Quelques autres endroits de ce genre:
- V612 Un «retour» inconditionnel dans une boucle. txtfrm.cxx 144
- V612 Un «retour» inconditionnel dans une boucle. txtfrm.cxx 202
- V612 Un «retour» inconditionnel dans une boucle. txtfrm.cxx 279
Des conditions étranges
V637 Deux conditions opposées ont été rencontrées. La deuxième condition est toujours fausse. Vérifiez les lignes: 281, 285. authfld.cxx 281
sal_uInt16 SwAuthorityFieldType::GetSequencePos(sal_IntPtr nHandle) { .... SwTOXSortTabBase* pOld = aSortArr[i].get(); if(*pOld == *pNew) {
L'analyseur a trouvé des comparaisons contradictoires. Quelque chose ne va clairement pas avec ce morceau de code.
Le même code est vu à cet endroit:
- V637 Deux conditions opposées ont été rencontrées. La deuxième condition est toujours fausse. Vérifiez les lignes: 1827, 1829. doctxm.cxx 1827
V590 Envisagez d'inspecter cette expression. L'expression est excessive ou contient une erreur d'impression. fileurl.cxx 55
OUString convertToFileUrl(char const * filename, ....) { .... if ((filename[0] == '.') || (filename[0] != SEPARATOR)) { .... } .... }
Le problème avec le fragment de code ci-dessus est que la première expression conditionnelle n'affecte pas le résultat de l'expression entière.
Sur la base de telles erreurs, j'ai même écrit un article théorique: "
Expressions logiques en C / C ++. Comment les professionnels se trompent ."
V590 Envisagez d'inspecter cette expression. L'expression est excessive ou contient une erreur d'impression. 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; } .... }
Je ne comprends pas immédiatement quel est le problème de cette condition, par conséquent, un fragment de code étendu a été écrit à partir du fichier prétraité:
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; }
Il se trouve que pas un seul nombre n'est inclus simultanément dans les 4 plages spécifiées dans la condition par des nombres. Les développeurs ont fait une erreur.
V590 Envisagez d'inspecter l'expression '* pData <= MAXLEVEL && * pData <= 9'. L'expression est excessive ou contient une erreur d'impression. ww8par2.cxx 756
const sal_uInt8 MAXLEVEL = 10; void SwWW8ImplReader::Read_ANLevelNo(....) { ....
Du fait que la première condition utilise une constante avec une valeur de
10 , la condition s'est avérée redondante. Ce morceau de code peut être réécrit comme suit:
if (*pData <= 9) { .... } else if( *pData == 10 || *pData == 11 ) { .... }
Mais il y avait peut-être un autre problème dans le code présenté.
V757 Il est possible qu'une variable incorrecte soit comparée à nullptr après la conversion de type en utilisant 'dynamic_cast'. Vérifiez les lignes: 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); }
Très probablement, la condition contient une erreur. Il a fallu vérifier les pointeurs
p et
pPopup .
V668 Il est inutile de tester le pointeur 'm_pStream' contre null, car la mémoire a été allouée à l'aide de l'opérateur 'new'. L'exception sera générée en cas d'erreur d'allocation de mémoire. 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; } }
L'analyseur a détecté une situation où la valeur du pointeur renvoyé par le
nouvel opérateur est comparée à zéro. Selon le standard du langage C ++, si l'allocation de mémoire n'est pas possible, le
nouvel opérateur
lèvera une exception
std :: bad_alloc . Seuls 45 de ces emplacements ont été trouvés dans le projet LibreOffice, ce qui est très petit pour un tel volume de code. Mais tout de même, cela peut entraîner des problèmes pour les utilisateurs. Les développeurs doivent supprimer les vérifications inutiles ou créer des objets de cette façon:
m_pStream = new (std::nothrow) FileStream(FileName.c_str());
V728 Un contrôle excessif peut être simplifié. Le '(A &&! B) || (! A && B) 'est équivalente à l'expression' 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) ||
Il y a très longtemps, les diagnostics
V728 ont été étendus aux cas qui, très probablement, ne sont pas erronés, mais compliquent le code. Et dans le code complexe, tôt ou tard, des erreurs sont encore commises.
Cette condition est simplifiée comme suit:
if (pItem->mbMirrorMode != bMirror) { .... }
Une soixantaine de structures similaires se trouvent dans le projet, certaines d'entre elles sont très, très volumineuses. Il est préférable que les auteurs du projet se familiarisent avec le rapport complet de l'analyseur PVS-Studio.
Problèmes de sécurité
V523 L'
instruction 'then' est équivalente à l'instruction '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); } }
Un très gros morceau de code est copié ici. Pour une fonction qui modifie certains droits, le problème identifié semble très suspect.
V1001 La variable 'DL' est affectée mais n'est pas utilisée à la fin de la fonction. 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; }
Les variables
DL et
DR sont annulées par une simple affectation à la fin de la fonction et ne sont plus utilisées. Pour le compilateur, c'est une excuse pour supprimer les commandes d'optimisation.
Pour nettoyer correctement les variables dans les applications Windows, vous pouvez réécrire le code de cette façon:
RtlSecureZeroMemory(&DL, sizeof(DL)); RtlSecureZeroMemory(&DR, sizeof(DR));
Mais pour LibreOffice, une solution multiplateforme est requise ici.
Un avertissement similaire d'une autre fonction:
- V1001 La variable 'DL' est affectée mais n'est pas utilisée à la fin de la fonction. cipher.cxx 860
V764 Ordre incorrect possible des arguments passés à la fonction 'queryStream': 'rUri' et '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 ); .... }
Veuillez noter que dans la fonction
queryStream , dans la liste des paramètres,
rPassword d' abord puis
rUri . Il y avait un endroit dans le code où les arguments correspondants étaient mélangés lorsque cette fonction était appelée.
V794 L'opérateur d'affectation doit être protégé contre le cas de 'this == & rToBeCopied'. hommatrixtemplate.hxx 121
ImplHomMatrixTemplate& operator=(....) {
Ce cas concerne davantage l'écriture sécurisée de code. Il n'y a pas de contrôle dans l'instruction de copie pour attribuer un objet à lui-même. Au total, le projet compte environ 30 implémentations de ce type de l'opérateur de copie.
Erreurs avec SysAllocString
V649 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la fonction return. Cela signifie que la deuxième déclaration «si» est insensée. Vérifiez les lignes: 125, 137. acctable.cxx 137
STDMETHODIMP CAccTable::get_columnDescription(long column, BSTR * description) { SolarMutexGuard g; ENTER_PROTECTED_BLOCK
La fonction
SysAllocString () renvoie un pointeur qui peut être
NULL . Quelque chose d'étrange a été écrit par l'auteur de ce code. Le pointeur vers la mémoire allouée n'est pas vérifié, ce qui peut entraîner des problèmes dans le programme.
Avertissements similaires provenant d'autres fonctions:
- V649 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la fonction return. Cela signifie que la deuxième déclaration «si» est insensée. Vérifiez les lignes: 344, 356. acctable.cxx 356
- V649 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la fonction return. Cela signifie que la deuxième déclaration «si» est insensée. Vérifier les lignes: 213, 219. trvlfrm.cxx 219
V530 La valeur de retour de la fonction 'SysAllocString' doit être utilisée. maccessible.cxx 1077
STDMETHODIMP CMAccessible::put_accValue(....) { .... if(varChild.lVal==CHILDID_SELF) { SysAllocString(m_pszValue); m_pszValue=SysAllocString(szValue); return S_OK; } .... }
Le résultat de l'un des appels à la fonction
SysAllocString () n'est pas utilisé. Les développeurs doivent prêter attention à ce morceau de code.
Divers
V716 Conversion de type suspect dans l'instruction return: a renvoyé HRESULT, mais la fonction renvoie en fait BOOL. maccessible.cxx 2649
BOOL CMAccessible::get_IAccessibleFromXAccessible(....) { ENTER_PROTECTED_BLOCK
L'une des branches de l'exécution de la fonction renvoie une valeur dont le type ne correspond pas au type de la valeur de retour de la fonction.
Le type HRESULT a un format plus complexe que le type BOOL et est conçu pour stocker les états d'opération. Par exemple, la valeur de E_INVALIDARG est 0x80070057L . Il sera correct d'écrire, par exemple, comme ceci: return FAILED(E_INVALIDARG);
Vous pouvez en savoir plus à ce sujet dans la documentation de diagnostic du V716.Quelques endroits plus similaires:- V716 Conversion de type suspect dans l'instruction return: a renvoyé HRESULT, mais la fonction renvoie en fait BOOL. inprocembobj.cxx 1299
- V716 Conversion de type suspect dans l'instruction return: a renvoyé HRESULT, mais la fonction renvoie en fait BOOL. maccessible.cxx 2660
V670 Le membre de classe non initialisé 'm_aMutex' est utilisé pour initialiser le membre 'm_aModifyListeners'. N'oubliez pas que les membres sont initialisés dans l'ordre de leurs déclarations à l'intérieur d'une 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) {
Selon la norme de langage, l'ordre d'initialisation des membres de classe dans le constructeur se produit dans l'ordre de leur déclaration dans la classe. Dans notre cas, le champ m_aMutex sera initialisé après avoir participé à l'initialisation de cinq autres champs de la classe.Voici à quoi ressemble le constructeur d'un des champs de classe: OInterfaceContainerHelper2( ::osl::Mutex & rMutex );
L'objet mutex est transmis par référence. Dans ce cas, divers problèmes peuvent survenir: de l'accès à la mémoire non initialisée à la perte ultérieure de modifications d'objets.V763 Le paramètre 'nNativeNumberMode' est toujours réécrit dans le corps de la fonction avant d'être utilisé. calendar_jewish.cxx 286 OUString SAL_CALL Calendar_jewish::getDisplayString(...., sal_Int16 nNativeNumberMode ) {
Les arguments de fonction sont remplacés pour diverses raisons: lutte contre les avertissements du compilateur, compatibilité descendante, béquille, etc. Cependant, aucune de ces solutions n'est bonne et le code semble déroutant.Vous devriez revoir le code de cet endroit et quelques autres similaires:- V763 Le paramètre 'bExtendedInfo' est toujours réécrit dans le corps de la fonction avant d'être utilisé. graphicfilter2.cxx 442
- V763 Le paramètre 'nVerbID' est toujours réécrit dans le corps de la fonction avant d'être utilisé. 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
Conclusion
Le désir de vérifier le code LibreOffice est apparu après mon utilisation personnelle du produit. Pour une raison quelconque, avec certains lancements aléatoires, le texte disparaît de tous les éléments de menu. Il ne reste que des icônes et des rayures monotones. L'erreur est probablement de haut niveau et, peut-être, elle ne peut pas être trouvée à l'aide d'outils d'analyse statique. Néanmoins, l'analyseur a trouvé beaucoup de problèmes qui ne sont pas liés à cela, et je serai heureux si les développeurs de LibreOffice prêtent attention aux analyseurs de code statiques et essaient de les utiliser pour améliorer la qualité et la fiabilité du projet. Ce sera utile à tout le monde.Merci de votre attention.
Abonnez-vous à nos chaînes et restez à l'écoute pour les nouvelles du monde de la programmation!
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Svyatoslav Razmyslov. LibreOffice: Cauchemar du comptable