Segfault caused by "Hook qt model for country"

[Lubomir I. Ivanov]

On 4 October 2017 at 02:22, Gaetan Bisson <bisson at archlinux.org> wrote:
> [2017-10-04 02:05:50 +0300] Lubomir I. Ivanov:
>> On 4 October 2017 at 01:46, Gaetan Bisson <bisson at archlinux.org> wrote:
>> > [2017-10-03 12:36:11 -1000] Gaetan Bisson:
>> >> [2017-10-03 10:56:48 -0700] Dirk Hohndel:
>> >> > On Mon, Oct 02, 2017 at 04:50:27PM -1000, Gaetan Bisson wrote:
>> >> > > Hi,
>> >> > >
>> >> > > I'm building the latest git snapshot of Subsurface on Arch Linux (we've
>> >> > > got qt-5.9.1) and the latest commit "Hook qt model for country"
>> >> > > (d5ab03ef4e84820f9530152dbf4ffc29dec6e652) causes a segfault.
>> >> > >
>> >> > > Building goes well but then when running the binary I get:
>> >> > >
>> >> > >   Thread 1 "subsurface" received signal SIGSEGV, Segmentation fault.
>> >> > >   0x0000555555721d87 in DiveListView::DiveListView(QWidget*) ()
>> >> > >   (gdb) bt
>> >> > >   #0  0x0000555555721d87 in DiveListView::DiveListView(QWidget*) ()
>> >> > >   #1  0x000055555566f07e in MainWindow::MainWindow() ()
>> >> > >   #2  0x000055555566858d in init_ui() ()
>> >> > >   #3  0x0000555555666f9d in main ()
>> >> > >
>> >> > > I've bisected this segfault to that precise commit. I have no idea how
>> >> > > to debug this further but will be happy to help in any way I can.
>> >> >
>> >> > Can you look at the disassembly at the crash and see if this might be
>> >> > similar to what Lubomir just reported? The loop going way further than it
>> >> > should?
>> >>
>> >> Yes. I've got no segfault after applying Lubomir's pull request.
>> >
>> > Though now, if the dive list is nonempty, the dive list widget shows
>> > *only* the location column.
>> >
>>
>> that's odd,
>> it doesn't happen for me.
>>
>> i made the default columns widths exactly the same as before.
>> can you provide a screenshot and also what happens if you resize the
>> columns to some normal widths and then restart subsurface?
>
> Sure. I've even tried erasing my Subsurface.conf file. The problem shows
> up as soon as there's one dive in the list. See attached. All is fine.
> Then I click on "Log ==> Add Dive" and all is still fine until I press
> "Apply changes" and then the dive list widget reloads with what you
> see...
>
> Adding some debugging printf's it appears the column width is originally
> set to the correct values. But as soon as dives are loaded into the
> widget, the width is asked again using columnWidth() which then returns
> zero for all columns...
>

Gaetan, can you please try applying the attached patch on the latest
master for reference?
what this patch does is move the list of widths in a QVector which is
in heap memory...
NOTE: this should not be a fix as the previous solution should have worked fine!

try adjusting the column widths if needed and try restarting Subsuface again.

lubomir
--
-------------- next part --------------
diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp
index 5777ae8..f115477 100644
--- a/qt-models/divetripmodel.cpp
+++ b/qt-models/divetripmodel.cpp
@@ -409,17 +409,21 @@ DiveTripModel::DiveTripModel(QObject *parent) :
 {
 	columns = COLUMNS;
 	// setup the default width of columns (px)
-	columnWidthMap = QVector<int>(COLUMNS);
+	columnWidthMap = new QVector<int>(COLUMNS);
 	// pre-fill with 50px; the rest are explicit
-	for(int i = 0; i < COLUMNS; i++)
-		columnWidthMap[i] = 50;
-	columnWidthMap[NR] = 70;
-	columnWidthMap[DATE] = 140;
-	columnWidthMap[RATING] = 90;
-	columnWidthMap[SUIT] = 70;
-	columnWidthMap[SAC] = 70;
-	columnWidthMap[PHOTOS] = 5;
-	columnWidthMap[LOCATION] = 500;
+	columnWidthMap->fill(50);
+	columnWidthMap->replace(NR, 70);
+	columnWidthMap->replace(DATE, 140);
+	columnWidthMap->replace(RATING, 90);
+	columnWidthMap->replace(SUIT, 70);
+	columnWidthMap->replace(SAC, 70);
+	columnWidthMap->replace(PHOTOS, 5);
+	columnWidthMap->replace(LOCATION, 500);
+}
+
+DiveTripModel::~DiveTripModel()
+{
+	delete columnWidthMap;
 }
 
 Qt::ItemFlags DiveTripModel::flags(const QModelIndex &index) const
@@ -625,7 +629,7 @@ int DiveTripModel::columnWidth(int column)
 		qWarning() << "DiveTripModel::columnWidth(): not a valid column index -" << column;
 		return 50;
 	}
-	return columnWidthMap[column];
+	return columnWidthMap->at(column);
 }
 
 void DiveTripModel::setColumnWidth(int column, int width)
@@ -634,5 +638,5 @@ void DiveTripModel::setColumnWidth(int column, int width)
 		qWarning() << "DiveTripModel::setColumnWidth(): not a valid column index -" << column;
 		return;
 	}
-	columnWidthMap[column] = width;
+	columnWidthMap->replace(column, width);
 }
diff --git a/qt-models/divetripmodel.h b/qt-models/divetripmodel.h
index 24b2151..74dbc34 100644
--- a/qt-models/divetripmodel.h
+++ b/qt-models/divetripmodel.h
@@ -92,6 +92,7 @@ public:
 	virtual QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const;
 	virtual bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::EditRole);
 	DiveTripModel(QObject *parent = 0);
+	~DiveTripModel();
 	Layout layout() const;
 	void setLayout(Layout layout);
 	int columnWidth(int column);
@@ -100,7 +101,7 @@ public:
 private:
 	void setupModelData();
 	QMap<dive_trip_t *, TripItem *> trips;
-	QVector<int> columnWidthMap;
+	QVector<int> *columnWidthMap;
 	Layout currentLayout;
 };