[PATCH] Be more careful about dive computer selection

Linus Torvalds torvalds at linux-foundation.org
Mon Mar 17 08:25:28 PDT 2014


From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Mon, 17 Mar 2014 08:19:09 -0700
Subject: [PATCH] Be more careful about dive computer selection

The selection logic was a bit random: some places would return NULL if
the dive computer index was out of range, others would return the
primary dive computer, and actually moving between dive computers would
just blindly increment and decrement the number.

This always selects the primary computer if the index is out of bounds,
and makes sure we stay in bound when switching beteen dive computers
(but switching between dives can then turn an in-bound number into an
out-of-bounds one)

Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---

This is largely the same patch as yesterday, except with a commit message 
and some type changes. I made "dc_number" (and the number_of_computers() 
function) explicitly unsigned, because the whole behavior with signed 
modulus is confusing.

 display.h                           |  4 ++--
 dive.h                              | 25 +++++++++++++++++++++----
 profile.c                           | 32 ++++++++------------------------
 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, 41 insertions(+), 38 deletions(-)

diff --git a/display.h b/display.h
index 35fa3749f2da..5af5c5612ac2 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 {
@@ -46,7 +46,7 @@ struct options {
 	int profile_height, notes_height, tanks_height;
 };
 
-extern char dc_number;
+extern unsigned int dc_number;
 
 extern unsigned int amount_selected;
 
diff --git a/dive.h b/dive.h
index c91fb03e405d..6c9a5b56633b 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 unsigned int number_of_computers(struct dive *dive)
+{
+	unsigned 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..2102179f23f4 100644
--- a/profile.c
+++ b/profile.c
@@ -16,7 +16,7 @@
 #include "membuffer.h"
 
 int selected_dive = -1; /* careful: 0 is a valid value */
-char dc_number = 0;
+unsigned int dc_number = 0;
 
 
 static struct plot_data *last_pi_entry_new = NULL;
@@ -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)
+struct divecomputer *select_dc(struct dive *dive)
 {
-	int i = 1;
-	struct divecomputer *dc = main;
+	unsigned int max = number_of_computers(dive);
+	unsigned int i = dc_number;
 
-	while ((dc = dc->next) != NULL)
-		i++;
-	return i;
-}
-
-struct divecomputer *select_dc(struct divecomputer *main)
-{
-	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 d5cf41981da8..247424ea185b 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);
-- 
1.9.0



More information about the subsurface mailing list