[REVIEW for 3.5] Fix redundant assignment of "nAngle" in switch in svx

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

[REVIEW for 3.5] Fix redundant assignment of "nAngle" in switch in svx

Hello,

In svx cppcheck reports this :
[source/items/algitem.cxx:183]: (warning) Redundant assignment of "nAngle" in switch
Here are the lines :
    switch( static_cast< SvxCellOrientation >( GetValue() ) )
    {
        case SVX_ORIENTATION_BOTTOMTOP: nAngle = 9000;
        case SVX_ORIENTATION_TOPBOTTOM: nAngle = 27000;
        default: ; //prevent warning
    }

It 's been there since the commit 56ce215d39efdb96703a53b7b51c9c5a0d61c9f1 (02/08/2004)

Can I cherry-pick this commit 49d7d4b3255f731ce9a8b5256da25f6a9bf53169 to 3.5 ?

Julien.
Tor Lillqvist-2 Tor Lillqvist-2
Reply | Threaded
Open this post in threaded view
|

Re: [REVIEW for 3.5] Fix redundant assignment of "nAngle" in switch in svx

> Can I cherry-pick this commit 49d7d4b3255f731ce9a8b5256da25f6a9bf53169 to
> 3.5 ?

Any idea what the visual impact of this change is? Who knows, maybe
the calling code actually expects the apparently nonsense semantics by
now;)

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

Re: [REVIEW for 3.5] Fix redundant assignment of "nAngle" in switch in svx

yep it creates fractal :-)
More seriously, I saw that it's at least used here in this file
sw/source/core/doc/tblafmt.cxx (line 413)

Julien.
Petr Mladek Petr Mladek
Reply | Threaded
Open this post in threaded view
|

Re: [REVIEW for 3.5] Fix redundant assignment of "nAngle" in switch in svx

julien2412 píše v Po 19. 03. 2012 v 14:40 -0700:
> yep it creates fractal :-)

You know, it is better to be careful. There are many workarounds for
bugs in the code. For example, there was a bug in widgets ordering. The
workaround made one widget transparent and passed events to the other
widget. Once we fixed the ordering, the other widget was suddenly
hidden. So the fixed bug caused regressions ;-)


> More seriously, I saw that it's at least used here in this file
> sw/source/core/doc/tblafmt.cxx (line 413)

Yup, I see the values used also svx/source/items/algitem.cxx in
SvxOrientationItem::SetFromRotation. I can't imagine any workaround for
this bug because the angle 9000 has newer been assigned. Your fix is
correct and looks safe => I have pushed it to 3-5 branch, see
http://cgit.freedesktop.org/libreoffice/core/commit/?h=libreoffice-3-5&id=85c6043d0d339d3df6a4037eedce78df38272ea6

You do great work!


Best Regards,
Petr

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

Re: [PUSHED:3-5] Fix redundant assignment of "nAngle" in switch in svx

Petr Mladek píše v Út 20. 03. 2012 v 09:49 +0100:
> Yup, I see the values used also svx/source/items/algitem.cxx in
> SvxOrientationItem::SetFromRotation. I can't imagine any workaround for
> this bug because the angle 9000 has newer been assigned. Your fix is
> correct and looks safe => I have pushed it to 3-5 branch, see
> http://cgit.freedesktop.org/libreoffice/core/commit/?h=libreoffice-3-5&id=85c6043d0d339d3df6a4037eedce78df38272ea6

Marked the thread as pushed.

Best Regards,
Petr

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