[Rosegarden-bugs] [ rosegarden-Bugs-3527808 ] cppcheck report

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

Bugs item #3527808, was opened at 2012-05-18 01:25
Message generated for change (Comment added) made by serval2412
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=104932&aid=3527808&group_id=4932

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: serval24 (serval2412)
Assigned to: Nobody/Anonymous (nobody)
Summary: cppcheck report

Initial Comment:
Hello,

I just svn checkout Rosegarden project and launched cppcheck (git updated today).
I thought it could interest you, here are some examples :
[src/gui/dialogs/IntervalDialog.cpp:239] -> [src/gui/dialogs/IntervalDialog.cpp:223]: (style) Found duplicate if expressions
    220         case 6: // seventh
    221            if (deviation == -1)
    222                textIntervalDeviated += tr("a minor");
    223            else if (deviation == 0)
    224                textIntervalDeviated += tr("a major");
    225            else if (deviation == -2)
    226                textIntervalDeviated += tr("a diminished");
    227            else if (deviation == 1)
    228                textIntervalDeviated += tr("an augmented");
    229            else if (deviation == -3)
    230                textIntervalDeviated += tr("a doubly diminished");
    231            else if (deviation == 2)
    232                textIntervalDeviated += tr("a doubly augmented");
    233            else if (deviation == -4)
    234                textIntervalDeviated += tr("a triply diminished");
    235            else if (deviation == 3)
    236                textIntervalDeviated += tr("a triply augmented");
    237            else if (deviation == 4)
    238                textIntervalDeviated += tr("a quadruply augmented");
    239            else if (deviation == 0)
    240                textIntervalDeviated += tr("a perfect");
    241            else
    242                textIntervalDeviated += tr("an (unknown, %1)").arg(deviation);
    243            break;

[src/gui/dialogs/ImportDeviceDialog.cpp:115]: (performance) Possible inefficient checking for 'm_devices' emptiness.
    115     if (m_devices.size() == 0) {

[src/gui/editors/eventlist/EventView.cpp:380]: (performance) Prefer prefix ++/-- operators for non-primitive types.
    379         for (Segment::iterator it = m_segments[i]->begin();
    380                 m_segments[i]->isBeforeEndMarker(it); it++) {

I attach the complete report. Even it certainly contains false reports, I just
thought it could help. If you prefer some patches, tell me. 

Julien.

----------------------------------------------------------------------

>Comment By: serval24 (serval2412)
Date: 2012-06-02 09:00

Message:
I attached the patch about emptiness.

About rerunning cppcheck, I'll do it later. But it's very easy, just git
clone, make, then use it this way :
cd <rosegarden_install_directory>trunk/rosegarden
<cppcheck_directory>/cppcheck --enable=all ./ 2>reports_cppcheck.txt

Julien

----------------------------------------------------------------------

Comment By: Ted Felix (tedfelix)
Date: 2012-05-25 16:19

Message:
That was still a lot of work to do.  Thanks.  I've committed your first
patch in r12943.  Looks like there may have been a few postfix
increment/decrements that were missed.  You might want to re-run cppcheck
and see if it finds any more.  Maybe they were basic types and cppcheck
ignored them.

----------------------------------------------------------------------

Comment By: serval24 (serval2412)
Date: 2012-05-25 05:39

Message:
Ok for the patch. About it, I tried to use the most automatic way, searched
about mass editing.
Badfully, I don't know enough about regexp so I generated a file to reduce
things to do.
I retrieved a file which contained things like :
<file> +<line>
With one console on this file and with another one, I copy pasted on with
"vi" (I'm on Linux) on prefix (+<line> allows to open the file directly on
the good line)
I think it must have taken about 1 hour once the file generated.

----------------------------------------------------------------------

Comment By: Ted Felix (tedfelix)
Date: 2012-05-25 05:29

Message:
I reviewed this patch.  It looks good.  Thanks for this.  Did you do all
this work by hand, or did you automate it in some way?

----------------------------------------------------------------------

Comment By: serval24 (serval2412)
Date: 2012-05-24 22:21

Message:
I attached the patch which is quite straightforward. Sorry for the delay.

Julien

----------------------------------------------------------------------

Comment By: Ted Felix (tedfelix)
Date: 2012-05-19 12:41

Message:
Patches would be great.  If you can focus first on the problems that are
easy to check by inspection (e.g. Prefer prefix ++/-- operators) that would
make it easier for us to evaluate and accept the patches.  For the harder
issues (e.g. the duplicate if expressions example) if you can provide a
test case that would exercise them along with the patch, that would be very
helpful.  Sometimes it is hard to tell how to exercise a piece of code as
rosegarden is very large.

And thanks for turning me on to cppcheck.  Looks like a handy tool.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=104932&aid=3527808&group_id=4932