2:28 p.m.
https://bugs.freedesktop.org/show_bug.cgi?id=72576
Priority: medium
Bug ID: 72576
CC: syncevolution-issues(a)syncevolution.org
Assignee: syncevolution-issues(a)syncevolution.org
Summary: src/dbus/server/server.cpp:495: possible bad if test ?
Severity: normal
Classification: Unclassified
OS: All
Reporter: dcb314(a)hotmail.com
Hardware: All
Status: NEW
Version: 1.3.99.3
Component: SyncEvolution
Product: SyncEvolution
I just ran the static analyser over syncevolution-1.3.99.3
It said many things, including
[src/dbus/server/server.cpp:495] -> [src/dbus/server/server.cpp:495]: (style)
Same expression on both sides of '<='.
Source code is
if (session && session->getPriority() <= session->getPriority())
{
Suggest code rework. I notice there is a parameter of the routine
called session and a local variable with the same name. Odd.
I also checked the newest source code and the bug appears there too.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.
3:15 p.m.
New subject: [Bug 72576] src/dbus/server/server.cpp:495: possible bad if test ?
https://bugs.freedesktop.org/show_bug.cgi?id=72576
Patrick Ohly <patrick.ohly(a)gmx.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|syncevolution-issues@syncev |patrick.ohly(a)gmx.de
|olution.org |
--- Comment #1 from Patrick Ohly <patrick.ohly(a)gmx.de> ---
Which code analyzer found that? I run Klocwork occasionally and analyzed all
problems in SyncEvolution code itself that Klocwork reported. In libsynthesis
the code triggered more warnings. The false negative rate was higher than in
SyncEvolution due to the different coding style, so I only checked some issues
there.
Anyway, I don't remember this one.
The code is the result of an invalid rewrite:
- if (it->lock()->getPriority() <= session->getPriority()) {
+ // skip over dead sessions, they will get cleaned up elsewhere
+ boost::shared_ptr<Session> session = it->lock();
+ if (session && session->getPriority() <= session->getPriority())
{
The session variable here shadows the session parameter, which clearly isn't
right. It's not that important, though, because session priorities aren't used
much (not at all?).
I'll fix it. Thanks for reporting this.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.
3:31 p.m.
New subject: [Bug 72576] src/dbus/server/server.cpp:495: possible bad if test ?
https://bugs.freedesktop.org/show_bug.cgi?id=72576
--- Comment #2 from dcb314(a)hotmail.com ---
Which code analyzer found that?
Sorry, I should have mentioned that.
cppcheck, as in http://cppcheck.sourceforge.net/
I find it a useful tool.
--
You are receiving this mail because:
You are on the CC list for the bug.
9:47 a.m.
New subject: [Bug 72576] src/dbus/server/server.cpp:495: possible bad if test ?
https://bugs.freedesktop.org/show_bug.cgi?id=72576
--- Comment #3 from Patrick Ohly <patrick.ohly(a)gmx.de> ---
(In reply to comment #2)
>Which code analyzer found that?
Sorry, I should have mentioned that.
cppcheck, as in http://cppcheck.sourceforge.net/
I find it a useful tool.
Agreed. I started applying it to libsynthesis, activesyncd and of course
SyncEvolution as part of the nightly testing. After some less important changes
(performance, unnecessary writes, style), a handful of suppressions and two
real fixes, the code passes. I'll notice regressions, too.
The other real problem was:
Author: Patrick Ohly <patrick.ohly(a)intel.com> 2014-01-07 11:04:22
Committer: Patrick Ohly <patrick.ohly(a)intel.com> 2014-01-17 15:35:55
Parent: 41f917d912c0a0a066e61ac07bd23492ca58af01 (testing: cppcheck redundant
assignment)
Child: a20a26df4b4ea3870d92982651ff0899bb595034 (cppcheck: suppress warnings)
Branch: for-master/testing
Follows: syncevolution-1-3-99-6
Precedes:
testing: fix naming of log files in doCopy()
The log file guard instance which should have added a "copy" part to log
files
was deleted again before it could have the desired effect. Found by
cppcheck
("Instance of 'SyncPrefix' object is destroyed immediately.").
----------------------------- test/ClientTest.cpp -----------------------------
index ab6a12f..e9d874d 100644
@@ -2977,9 +2977,9 @@ void SyncTests::deleteAll(DeleteAllMode mode) {
}
/** get both clients in sync with empty server, then copy one item from client
A to B */
void SyncTests::doCopy() {
- SyncPrefix("copy", *this);
+ SyncPrefix copy("copy", *this);
// check requirements
CT_ASSERT(accessClientB);
--
You are receiving this mail because:
You are on the CC list for the bug.
3:37 a.m.
New subject: [Bug 72576] src/dbus/server/server.cpp:495: possible bad if test ?
https://bugs.freedesktop.org/show_bug.cgi?id=72576
Patrick Ohly <patrick.ohly(a)gmx.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #4 from Patrick Ohly <patrick.ohly(a)gmx.de> ---
Fixes and cppcheck-enhanced test scripts were merged into master.
--
You are receiving this mail because:
You are on the CC list for the bug.
2599
days inactive
2640
days old
syncevolution-issues@syncevolution.org
4 comments
1 participants
participants (1)
-
bugzilla-daemon＠freedesktop.org