Some subsurface notes from a week of diving

Linus Torvalds torvalds at linux-foundation.org
Sun Mar 16 15:52:25 PDT 2014


On Sun, Mar 16, 2014 at 1:57 PM, Dirk Hohndel <dirk at hohndel.org> wrote:
>
> Oops. We used to have code that fixes that. Wonder where that got lost.

I think the new profile lost it in the re-implementation. The old code
didn't blindly just update the dc number.

Blindly updating the dc number is wrong for other reasons too. Even if
we do modulus arithmetic when looking up the dive computer (so that
"--" and "++" just does the right thing), the overflow in the type
(which is just a char) would make the modulus come out wrong when it
overflows. It also makes it impossible to reset sanely when switching
dives (unless you just *always* reset to zero when switching dives,
but it's actually nice to walk dives and see your "secondary" dive
computer the whole time if you have a consistent ordering, without
having to switch dives and then always switch to the secondary
explicitly).

But even if that code is made to do the proper modulus at update, we
*also* have to be careful when we've switched dives, and the new dive
has fewer dive computers.

So I think we need to do both, just to be safe: don't blindly update
dc numbers, but on use we need to also check the dive number against
the number of dive computers and reset it to zero if it's beyond the
end (and there we don't want to do modulus, we really want to just say
"out or range means 'reset to first'").

The attached patch does something like that. I haven't tested it
extensively. And I changed the calling convention of "select_dc()" to
take the "struct dive" instead: that's what every single caller
wanted, and that way it matches (and can just use) the very similar
"get_dive_dc()".

So the new rules are:
 - left/right cursor needs to do the proper modulo arithmetic
 - number_of_computers() returns the obvious value, except that if
"dive" is NULL, it still returns 1 so that you can do the modulo thing
without worrying about divide-by-zero even if there is no current dive
 - get_dive_dc() (and thus "current_dc") will just return the primary
dc if the index is out of range
 - select_dc() does the same thing, but additionally resets dc_number
to zero for the out-of-range case.

Dirk, consider this signed-off-by-me, but it might want some more
testing. I tested it very minimally.

                       Linus
-------------- next part --------------
 display.h                           |  2 +-
 dive.h                              | 25 +++++++++++++++++++++----
 profile.c                           | 28 ++++++----------------------
 qt-ui/diveplanner.cpp               |  2 +-
 qt-ui/maintab.cpp                   |  6 +++---
 qt-ui/mainwindow.cpp                |  6 ++++--
 qt-ui/profile/diveplotdatamodel.cpp |  2 +-
 qt-ui/profile/profilewidget2.cpp    |  2 +-
 8 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/display.h b/display.h
index 35fa3749f2da..10725c9fafc4 100644
--- a/display.h
+++ b/display.h
@@ -32,7 +32,7 @@ typedef enum {
 	SC_PRINT
 } scale_mode_t;
 
-extern struct divecomputer *select_dc(struct divecomputer *main);
+extern struct divecomputer *select_dc(struct dive *);
 
 struct options {
 	enum {
diff --git a/dive.h b/dive.h
index c91fb03e405d..2640479e3c63 100644
--- a/dive.h
+++ b/dive.h
@@ -600,13 +600,30 @@ static inline struct dive *get_dive(int nr)
 	return dive_table.dives[nr];
 }
 
+static inline int number_of_computers(struct dive *dive)
+{
+	int total_number = 0;
+	struct divecomputer *dc = &dive->dc;
+
+	if (!dive)
+		return 1;
+
+	do {
+		total_number++;
+		dc = dc->next;
+	} while (dc);
+	return total_number;
+}
+
 static inline struct divecomputer *get_dive_dc(struct dive *dive, int nr)
 {
-	struct divecomputer *dc = NULL;
-	if (nr >= 0)
-		dc = &dive->dc;
-	while (nr-- > 0)
+	struct divecomputer *dc = &dive->dc;
+
+	while (nr-- > 0) {
 		dc = dc->next;
+		if (!dc)
+			return &dive->dc;
+	}
 	return dc;
 }
 
diff --git a/profile.c b/profile.c
index 6cf137edb9dc..eca8feb94413 100644
--- a/profile.c
+++ b/profile.c
@@ -1193,32 +1193,16 @@ void create_plot_info_new(struct dive *dive, struct divecomputer *dc, struct plo
 	analyze_plot_info(pi);
 }
 
-/* make sure you pass this the FIRST dc - it just walks the list */
-static int nr_dcs(struct divecomputer *main)
-{
-	int i = 1;
-	struct divecomputer *dc = main;
-
-	while ((dc = dc->next) != NULL)
-		i++;
-	return i;
-}
-
-struct divecomputer *select_dc(struct divecomputer *main)
+struct divecomputer *select_dc(struct dive *dive)
 {
+	int max = number_of_computers(dive);
 	int i = dc_number;
-	struct divecomputer *dc = main;
 
-	while (i < 0)
-		i += nr_dcs(main);
-	do {
-		if (--i < 0)
-			return dc;
-	} while ((dc = dc->next) != NULL);
+	/* Reset 'dc_number' if we've switched dives and it is now out of range */
+	if (i >= max)
+		dc_number = i = 0;
 
-	/* If we switched dives to one with fewer DC's, reset the dive computer counter */
-	dc_number = 0;
-	return main;
+	return get_dive_dc(dive, i);
 }
 
 static void plot_string(struct plot_data *entry, struct membuffer *b, bool has_ndl)
diff --git a/qt-ui/diveplanner.cpp b/qt-ui/diveplanner.cpp
index 27eedb58652e..476812080cd2 100644
--- a/qt-ui/diveplanner.cpp
+++ b/qt-ui/diveplanner.cpp
@@ -1472,7 +1472,7 @@ void DivePlannerPointsModel::createPlan()
 	plan(&diveplan, &cache, &tempDive, isPlanner());
 	copy_cylinders(stagingDive, tempDive);
 	int mean[MAX_CYLINDERS], duration[MAX_CYLINDERS];
-	per_cylinder_mean_depth(tempDive, select_dc(&tempDive->dc), mean, duration);
+	per_cylinder_mean_depth(tempDive, select_dc(tempDive), mean, duration);
 	for (int i = 0; i < MAX_CYLINDERS; i++) {
 		cylinder_t *cyl = tempDive->cylinder + i;
 		if (cylinder_none(cyl))
diff --git a/qt-ui/maintab.cpp b/qt-ui/maintab.cpp
index 378ee61634a6..660fc4f90a43 100644
--- a/qt-ui/maintab.cpp
+++ b/qt-ui/maintab.cpp
@@ -483,7 +483,7 @@ void MainTab::updateDiveInfo(int dive)
 		get_gas_used(d, gases);
 		QString volumes = get_volume_string(gases[0], true);
 		int mean[MAX_CYLINDERS], duration[MAX_CYLINDERS];
-		per_cylinder_mean_depth(d, select_dc(&d->dc), mean, duration);
+		per_cylinder_mean_depth(d, select_dc(d), mean, duration);
 		volume_t sac;
 		QString SACs;
 		if (mean[0] && duration[0]) {
@@ -864,13 +864,13 @@ void MainTab::on_divemaster_textChanged()
 
 void MainTab::on_airtemp_textChanged(const QString &text)
 {
-	EDIT_SELECTED_DIVES(select_dc(&mydive->dc)->airtemp.mkelvin = parseTemperatureToMkelvin(text));
+	EDIT_SELECTED_DIVES(select_dc(mydive)->airtemp.mkelvin = parseTemperatureToMkelvin(text));
 	markChangedWidget(ui.airtemp);
 }
 
 void MainTab::on_watertemp_textChanged(const QString &text)
 {
-	EDIT_SELECTED_DIVES(select_dc(&mydive->dc)->watertemp.mkelvin = parseTemperatureToMkelvin(text));
+	EDIT_SELECTED_DIVES(select_dc(mydive)->watertemp.mkelvin = parseTemperatureToMkelvin(text));
 	markChangedWidget(ui.watertemp);
 }
 
diff --git a/qt-ui/mainwindow.cpp b/qt-ui/mainwindow.cpp
index 02c424fe26ce..8ca3bfad3cb9 100644
--- a/qt-ui/mainwindow.cpp
+++ b/qt-ui/mainwindow.cpp
@@ -507,14 +507,16 @@ void MainWindow::saveSplitterSizes()
 
 void MainWindow::on_actionPreviousDC_triggered()
 {
-	dc_number--;
+	unsigned nrdc = number_of_computers(current_dive);
+	dc_number = (dc_number + nrdc - 1) % nrdc;
 	ui.InfoWidget->updateDiveInfo(selected_dive);
 	ui.newProfile->plotDives(QList<struct dive *>() << (current_dive));
 }
 
 void MainWindow::on_actionNextDC_triggered()
 {
-	dc_number++;
+	unsigned nrdc = number_of_computers(current_dive);
+	dc_number = (dc_number + 1) % nrdc;
 	ui.InfoWidget->updateDiveInfo(selected_dive);
 	ui.newProfile->plotDives(QList<struct dive *>() << (current_dive));
 }
diff --git a/qt-ui/profile/diveplotdatamodel.cpp b/qt-ui/profile/diveplotdatamodel.cpp
index 7412a77641c6..755dd9264555 100644
--- a/qt-ui/profile/diveplotdatamodel.cpp
+++ b/qt-ui/profile/diveplotdatamodel.cpp
@@ -183,7 +183,7 @@ void DivePlotDataModel::calculateDecompression()
 	struct dive *d = getDiveById(id());
 	if (!d)
 		return;
-	struct divecomputer *dc = select_dc(&d->dc);
+	struct divecomputer *dc = select_dc(d);
 	init_decompression(d);
 	calculate_deco_information(d, dc, &pInfo, false);
 	dataChanged(index(0, CEILING), index(pInfo.nr - 1, TISSUE_16));
diff --git a/qt-ui/profile/profilewidget2.cpp b/qt-ui/profile/profilewidget2.cpp
index 7e0aba001bc7..f8c7dfc6c23b 100644
--- a/qt-ui/profile/profilewidget2.cpp
+++ b/qt-ui/profile/profilewidget2.cpp
@@ -358,7 +358,7 @@ void ProfileWidget2::plotDives(QList<dive *> dives)
 	// next get the dive computer structure - if there are no samples
 	// let's create a fake profile that's somewhat reasonable for the
 	// data that we have
-	struct divecomputer *currentdc = select_dc(&d->dc);
+	struct divecomputer *currentdc = select_dc(d);
 	Q_ASSERT(currentdc);
 	if (!currentdc || !currentdc->samples) {
 		currentdc = fake_dc(currentdc);


More information about the subsurface mailing list