[Libreoffice] cppcheck "Found duplicate branches for if and else" in libs-core

[David Tardon]

On Fri, Apr 22, 2011 at 02:06:54PM +0200, Julien Nabet wrote:
> Hello,

Hi,

many thanks for your hard work!

> I've runned a cppcheck last git version on libs-core (master branch
> updated today) and found this oddity :
> [basic/source/app/mybasic.cxx:265] ->
> [basic/source/app/mybasic.cxx:260]: (style) Found duplicate branches
> for if and else.

It seems it has been like this since forever :) There were two other,
commented out, lines in the if branch originally; I suppose they were
removed as part of our "cleaning up" easy hacks. I just moved the return
up and removed the--now useless--if statement.

> [embeddedobj/source/msole/olepersist.cxx:1835] ->
> [embeddedobj/source/msole/olepersist.cxx:1823]: (style) Found

This is the same case as above, except that there is a comment
suggesting that the m_bIsLink case might need special treatment. I
simplified it nevertheless and added a comment to that effect.

> duplicate branches for if and else.
> [svx/source/gallery2/galctrl.cxx:701] ->
> [svx/source/gallery2/galctrl.cxx:699]: (style) Found duplicate
> branches for if and else.

Still the same story... The else branch was changed 10 (!) years ago to
assign the same value as the if branch...

> [svx/source/svdraw/svdpagv.cxx:890] ->
> [svx/source/svdraw/svdpagv.cxx:887]: (style) Found duplicate
> branches for if and else.

Should I repeat myself? :) There is a comment saying "Hier optimieren ..."
in the if branch originating in 2000, but the code has not been touched
since...

> [svx/source/unodraw/unoshape.cxx:3091] ->
> [svx/source/unodraw/unoshape.cxx:3084]: (style) Found duplicate
> branches for if and else.

I thought this is a bug at first, but getPropertyState already does the
right thing :) Anyway, since this if (mpImpl->mpMaster) ... else ... is
pattern the other functions follow, I just changed the impl. so the
microoptimization actually might have some effect.

> 
> Is it a kind of "side effect" of a merge ?

It is not, as you can see from above comments. Every one of these
annoyances has been with us for a looong time :)

> Then I don't know if the "if/else" must be removed or if one of them
> must be modified ; I let you judge.
> If useful, I can run cppcheck on the other modules and filter on
> this "style" error, just tell me.

Well, this really depends on your willingness to do it :) I suppose they
are still listed on your cppcheck webpage, so if anyone feels like
incinerating a few useless lines of code he can find them there?

D.