cppcheck: duplicate branch for ParaPropertyPanel (svx) + interpr7.cxx (sc)

classic Classic list List threaded Threaded
4 messages Options
julien2412 julien2412
Reply | Threaded
Open this post in threaded view
|

cppcheck: duplicate branch for ParaPropertyPanel (svx) + interpr7.cxx (sc)

Hello,

Cppcheck reported this:
sc/source/core/tool/interpr7.cxx
94 duplicateBranch style Found duplicate branches for 'if' and 'else'.

     87                         if(pNodeSet->nodeTab[0]->type == XML_NAMESPACE_DECL)
     88                         {
     89                             xmlNsPtr ns = (xmlNsPtr)pNodeSet->nodeTab[0];
     90                             xmlNodePtr cur = (xmlNodePtr)ns->next;
     91                             boost::shared_ptr<xmlChar> pChar2(xmlNodeGetContent(cur), xmlFree);
     92                             aResult = OUString::createFromAscii((char*)pChar2.get());
     93                         }
     94                         else if(pNodeSet->nodeTab[0]->type == XML_ELEMENT_NODE)
     95                         {
     96                             xmlNodePtr cur = pNodeSet->nodeTab[0];
     97                             boost::shared_ptr<xmlChar> pChar2(xmlNodeGetContent(cur), xmlFree);
     98                             aResult = OUString::createFromAscii((char*)pChar2.get());
     99                         }
    100                         else
    101                         {
    102                             xmlNodePtr cur = pNodeSet->nodeTab[0];
    103                             boost::shared_ptr<xmlChar> pChar2(xmlNodeGetContent(cur), xmlFree);
    104                             aResult = OUString::createFromAscii((char*)pChar2.get());
    105                         }

See http://opengrok.libreoffice.org/xref/core/sc/source/core/tool/interpr7.cxx#87

and this:
svx/source/sidebar/paragraph/ParaPropertyPanel.cxx
309 duplicateBranch style Found duplicate branches for 'if' and 'else'.
328 duplicateBranch style Found duplicate branches for 'if' and 'else'.
Indeed:
    309     if( Application::GetSettings().GetLayoutRTL())
    310     {
    311         mpTbxIndent_IncDec->SetItemImage(nIdIncrement, maIncIndentControl.GetIcon());
    312         mpTbxIndent_IncDec->SetItemImage(nIdDecrement, maDecIndentControl.GetIcon());
    313     }
    314     else
    315     {
    316         mpTbxIndent_IncDec->SetItemImage(nIdIncrement, maIncIndentControl.GetIcon());
    317         mpTbxIndent_IncDec->SetItemImage(nIdDecrement, maDecIndentControl.GetIcon());
    318     }

(the same for part around line 328)
See http://opengrok.libreoffice.org/xref/core/svx/source/sidebar/paragraph/ParaPropertyPanel.cxx#309

I suppose it should be fixed but I don't know how.
Any idea?

Julien
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: cppcheck: duplicate branch for ParaPropertyPanel (svx)

On 28.09.2013 01:52, julien2412 wrote:

> svx/source/sidebar/paragraph/ParaPropertyPanel.cxx
> 309 duplicateBranch style Found duplicate branches for 'if' and 'else'.
> 328 duplicateBranch style Found duplicate branches for 'if' and 'else'.
> Indeed:
>     309     if( Application::GetSettings().GetLayoutRTL())
>     310     {
>     311         mpTbxIndent_IncDec->SetItemImage(nIdIncrement,
> maIncIndentControl.GetIcon());
>     312         mpTbxIndent_IncDec->SetItemImage(nIdDecrement,
> maDecIndentControl.GetIcon());
>     313     }
>     314     else
>     315     {
>     316         mpTbxIndent_IncDec->SetItemImage(nIdIncrement,
> maIncIndentControl.GetIcon());
>     317         mpTbxIndent_IncDec->SetItemImage(nIdDecrement,
> maDecIndentControl.GetIcon());
>     318     }
>
> (the same for part around line 328)
> See
> http://opengrok.libreoffice.org/xref/core/svx/source/sidebar/paragraph/ParaPropertyPanel.cxx#309

Hi Julien,

The condition does not make sense - it should check for a text
direction, not UI direction. The code itself does not make sense too -
the images are swapped, but the tooltips are not... :)

So you can remove the duplicate branches.

Regards,
Ivan


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: cppcheck: duplicate branch for ParaPropertyPanel (svx)

On 28.09.2013 10:57, Ivan Timofeev wrote:
> The code itself does not make sense too -
> the images are swapped, but the tooltips are not... :)

please ignore this brainfart, oh...
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Caolán McNamara Caolán McNamara
Reply | Threaded
Open this post in threaded view
|

Re: cppcheck: duplicate branch for ParaPropertyPanel (svx) + interpr7.cxx (sc)

In reply to this post by julien2412
On Fri, 2013-09-27 at 14:52 -0700, julien2412 wrote:

> Hello,
>
> Cppcheck reported this:
> sc/source/core/tool/interpr7.cxx
>      94                         else if(pNodeSet->nodeTab[0]->type ==
> XML_ELEMENT_NODE)
>      95                         {
>      96                             xmlNodePtr cur = pNodeSet->nodeTab[0];
>      97                             boost::shared_ptr<xmlChar>
> pChar2(xmlNodeGetContent(cur), xmlFree);
>      98                             aResult =
> OUString::createFromAscii((char*)pChar2.get());
>      99                         }
>     100                         else
>     101                         {
>     102                             xmlNodePtr cur = pNodeSet->nodeTab[0];
>     103                             boost::shared_ptr<xmlChar>
> pChar2(xmlNodeGetContent(cur), xmlFree);
>     104                             aResult =
> OUString::createFromAscii((char*)pChar2.get());
>     105                         }
>
> See
> http://opengrok.libreoffice.org/xref/core/sc/source/core/tool/interpr7.cxx#87

So I decided to just remove the duplication for now. Markus might want
to have a look at it as the original author.

C.

(and the other issue in svx is fixed now as well as per the earlier
mail).

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice