Merge lp:~zsombi/ubuntu-ui-toolkit/alarmmodel-segfault into lp:ubuntu-ui-toolkit/staging

Proposed by Cris Dywan
Status: Merged
Approved by: Zsombor Egri
Approved revision: 1197
Merged at revision: 1549
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/alarmmodel-segfault
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 47 lines (+12/-2)
2 files modified
modules/Ubuntu/Components/plugin/ucalarmmodel.cpp (+9/-2)
modules/Ubuntu/Components/plugin/ucalarmmodel.h (+3/-0)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/alarmmodel-segfault
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Zsombor Egri Approve
Nekhelesh Ramananthan Pending
Tim Peeters Pending
Review via email: mp+263940@code.launchpad.net

This proposal supersedes a proposal from 2015-06-16.

Commit message

Fix AlamrModel segfault when updating elements.

To post a comment you must log in.
Revision history for this message
Tim Peeters (tpeeters) wrote : Posted in a previous version of this proposal

Seems good.

Including a regression test would be even better, but may be difficult to write.

review: Approve
Revision history for this message
Tim Peeters (tpeeters) wrote : Posted in a previous version of this proposal

39 +private:
40 + bool m_moved:1;

why not initialize it with 'false'?

Revision history for this message
Zsombor Egri (zsombi) wrote : Posted in a previous version of this proposal

> 39 +private:
> 40 + bool m_moved:1;
>
> why not initialize it with 'false'?

Uhm... it is initialised with false...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote : Posted in a previous version of this proposal

On testing on Nexus 4 ubuntu-touch/rc-proposed/ubuntu r162, I am unable to reproduce the crash after applying this patch. This might just be it :). That said, this crash is pretty rare and difficult to reproduce and so I have been trying hard to ensure that it is gone for good. But there could be a small chance that it may still exist (purely from testing).

With that I'm giving my approval.

review: Approve
Revision history for this message
Tim Peeters (tpeeters) wrote : Posted in a previous version of this proposal

Good to land, but it needs to be manually included in a landing or retargeted to staging.

review: Approve
Revision history for this message
Zsombor Egri (zsombi) wrote :

Approved :)

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'modules/Ubuntu/Components/plugin/ucalarmmodel.cpp'
2--- modules/Ubuntu/Components/plugin/ucalarmmodel.cpp 2015-03-24 16:51:39 +0000
3+++ modules/Ubuntu/Components/plugin/ucalarmmodel.cpp 2015-07-06 17:07:38 +0000
4@@ -125,6 +125,7 @@
5
6 UCAlarmModel::UCAlarmModel(QObject *parent)
7 : QAbstractListModel(parent)
8+ , m_moved(false)
9 {
10 // keep in sync with alarms collection changes
11 // some of the connections can be asynchronous, others synchronous
12@@ -318,7 +319,10 @@
13 */
14 void UCAlarmModel::moveStarted(int from, int to)
15 {
16- beginMoveRows(QModelIndex(), from, from, QModelIndex(), to);
17+ if (m_moved) {
18+ return;
19+ }
20+ m_moved = beginMoveRows(QModelIndex(), from, from, QModelIndex(), to);
21 }
22
23 /*!
24@@ -327,5 +331,8 @@
25 */
26 void UCAlarmModel::moveFinished()
27 {
28- endMoveRows();
29+ if (m_moved) {
30+ endMoveRows();
31+ }
32+ m_moved = false;
33 }
34
35=== modified file 'modules/Ubuntu/Components/plugin/ucalarmmodel.h'
36--- modules/Ubuntu/Components/plugin/ucalarmmodel.h 2014-10-20 12:49:10 +0000
37+++ modules/Ubuntu/Components/plugin/ucalarmmodel.h 2015-07-06 17:07:38 +0000
38@@ -63,6 +63,9 @@
39 void insertFinished();
40 void moveStarted(int from, int to);
41 void moveFinished();
42+
43+private:
44+ bool m_moved:1;
45 };
46
47 #endif // UCALARMSMODEL_H

Subscribers

People subscribed via source and target branches