Cylinders: access cylinders with get_cylinder()

Instead of accessing the cylinder table directly, use the get_cylinder()
function. This gives less unwieldy expressions. But more importantly,
the function does bound checking. This is crucial for now as the code
hasn't be properly audited since the change to arbitrarily sized
cylinder tables. Accesses of invalid cylinder indexes may lead to
silent data-corruption that is sometimes not even noticed by
valgrind. Returning NULL instead of an invalid pointer will make
debugging much easier.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

4 files changed, 16 insertions, 16 deletions

diff --git a/qt-models/cylindermodel.cpp b/qt-models/cylindermodel.cpp
index f64cc213a..22fc5ae8b 100644
--- a/qt-models/cylindermodel.cpp
+++ b/qt-models/cylindermodel.cpp
@@ -134,7 +134,7 @@ QVariant CylindersModel::data(const QModelIndex &index, int role) const
 	if (!index.isValid() || index.row() >= rows)
 		return QVariant();
 
-	const cylinder_t *cyl = &displayed_dive.cylinders.cylinders[index.row()];
+	const cylinder_t *cyl = get_cylinder(&displayed_dive, index.row());
 
 	switch (role) {
 	case Qt::BackgroundRole: {
@@ -259,7 +259,7 @@ QVariant CylindersModel::data(const QModelIndex &index, int role) const
 
 cylinder_t *CylindersModel::cylinderAt(const QModelIndex &index)
 {
-	return &displayed_dive.cylinders.cylinders[index.row()];
+	return get_cylinder(&displayed_dive, index.row());
 }
 
 // this is our magic 'pass data in' function that allows the delegate to get
@@ -449,7 +449,7 @@ static bool show_cylinder(struct dive *dive, int i)
 	if (is_cylinder_used(dive, i))
 		return true;
 
-	cylinder_t *cyl = dive->cylinders.cylinders + i;
+	cylinder_t *cyl = get_cylinder(dive, i);
 	if (cyl->start.mbar || cyl->sample_start.mbar ||
 	    cyl->end.mbar || cyl->sample_end.mbar)
 		return true;
@@ -533,10 +533,10 @@ void CylindersModel::moveAtFirst(int cylid)
 	cylinder_t temp_cyl;
 
 	beginMoveRows(QModelIndex(), cylid, cylid, QModelIndex(), 0);
-	memmove(&temp_cyl, &displayed_dive.cylinders.cylinders[cylid], sizeof(temp_cyl));
+	memmove(&temp_cyl, get_cylinder(&displayed_dive, cylid), sizeof(temp_cyl));
 	for (int i = cylid - 1; i >= 0; i--)
-		memmove(&displayed_dive.cylinders.cylinders[i + 1], &displayed_dive.cylinders.cylinders[i], sizeof(temp_cyl));
-	memmove(&displayed_dive.cylinders.cylinders[0], &temp_cyl, sizeof(temp_cyl));
+		memmove(get_cylinder(&displayed_dive, i + 1), get_cylinder(&displayed_dive, i), sizeof(temp_cyl));
+	memmove(get_cylinder(&displayed_dive, 0), &temp_cyl, sizeof(temp_cyl));
 
 	// Create a mapping of cylinder indexes:
 	// 1) Fill mapping[0]..mapping[cyl] with 0..index
@@ -558,7 +558,7 @@ void CylindersModel::updateDecoDepths(pressure_t olddecopo2)
 	pressure_t decopo2;
 	decopo2.mbar = prefs.decopo2;
 	for (int i = 0; i < displayed_dive.cylinders.nr; i++) {
-		cylinder_t *cyl = &displayed_dive.cylinders.cylinders[i];
+		cylinder_t *cyl = get_cylinder(&displayed_dive, i);
 		/* If the gas's deco MOD matches the old pO2, it will have been automatically calculated and should be updated.
 		 * If they don't match, we should leave the user entered depth as it is */
 		if (cyl->depth.mm == gas_mod(cyl->gasmix, olddecopo2, &displayed_dive, M_OR_FT(3, 10)).mm) {
@@ -578,7 +578,7 @@ bool CylindersModel::updateBestMixes()
 	// Check if any of the cylinders are best mixes, update if needed
 	bool gasUpdated = false;
 	for (int i = 0; i < displayed_dive.cylinders.nr; i++) {
-		cylinder_t *cyl = &displayed_dive.cylinders.cylinders[i];
+		cylinder_t *cyl = get_cylinder(&displayed_dive, i);
 		if (cyl->bestmix_o2) {
 			cyl->gasmix.o2 = best_o2(displayed_dive.maxdepth, &displayed_dive);
 			// fO2 + fHe must not be greater than 1
diff --git a/qt-models/diveplannermodel.cpp b/qt-models/diveplannermodel.cpp
index 8d19afcfb..dbac82681 100644
--- a/qt-models/diveplannermodel.cpp
+++ b/qt-models/diveplannermodel.cpp
@@ -268,13 +268,13 @@ QVariant DivePlannerPointsModel::data(const QModelIndex &index, int role) const
 		case GAS:
 			/* Check if we have the same gasmix two or more times
 			 * If yes return more verbose string */
-			int same_gas = same_gasmix_cylinder(&displayed_dive.cylinders.cylinders[p.cylinderid], p.cylinderid, &displayed_dive, true);
+			int same_gas = same_gasmix_cylinder(get_cylinder(&displayed_dive, p.cylinderid), p.cylinderid, &displayed_dive, true);
 			if (same_gas == -1)
-				return get_gas_string(displayed_dive.cylinders.cylinders[p.cylinderid].gasmix);
+				return get_gas_string(get_cylinder(&displayed_dive, p.cylinderid)->gasmix);
 			else
-				return get_gas_string(displayed_dive.cylinders.cylinders[p.cylinderid].gasmix) +
+				return get_gas_string(get_cylinder(&displayed_dive, p.cylinderid)->gasmix) +
 					QString(" (%1 %2 ").arg(tr("cyl.")).arg(p.cylinderid + 1) +
-						displayed_dive.cylinders.cylinders[p.cylinderid].type.description + ")";
+						get_cylinder(&displayed_dive, p.cylinderid)->type.description + ")";
 		}
 	} else if (role == Qt::DecorationRole) {
 		switch (index.column()) {
@@ -894,7 +894,7 @@ void DivePlannerPointsModel::createTemporaryPlan()
 	struct deco_state *cache = NULL;
 	struct divedatapoint *dp = NULL;
 	for (int i = 0; i < displayed_dive.cylinders.nr; i++) {
-		cylinder_t *cyl = &displayed_dive.cylinders.cylinders[i];
+		cylinder_t *cyl = get_cylinder(&displayed_dive, i);
 		if (cyl->depth.mm && cyl->cylinder_use != NOT_USED) {
 			dp = create_dp(0, cyl->depth.mm, i, 0);
 			if (diveplan.dp) {
diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp
index 220d122d4..ada648762 100644
--- a/qt-models/divetripmodel.cpp
+++ b/qt-models/divetripmodel.cpp
@@ -157,7 +157,7 @@ QVariant DiveTripModelBase::diveData(const struct dive *d, int column, int role)
 		case SUIT:
 			return QString(d->suit);
 		case CYLINDER:
-			return d->cylinders.nr > 0 ? QString(d->cylinders.cylinders[0].type.description) : QString();
+			return d->cylinders.nr > 0 ? QString(get_cylinder(d, 0)->type.description) : QString();
 		case SAC:
 			return displaySac(d, prefs.units.show_units_table);
 		case OTU:
@@ -1411,7 +1411,7 @@ bool DiveTripModelList::lessThan(const QModelIndex &i1, const QModelIndex &i2) c
 		return lessThanHelper(strCmp(d1->suit, d2->suit), row_diff);
 	case CYLINDER:
 		if (d1->cylinders.nr > 0 && d2->cylinders.nr > 0)
-			return lessThanHelper(strCmp(d1->cylinders.cylinders[0].type.description, d2->cylinders.cylinders[0].type.description), row_diff);
+			return lessThanHelper(strCmp(get_cylinder(d1, 0)->type.description, get_cylinder(d2, 0)->type.description), row_diff);
 		return d1->cylinders.nr - d2->cylinders.nr < 0;
 	case GAS:
 		return lessThanHelper(nitrox_sort_value(d1) - nitrox_sort_value(d2), row_diff);
diff --git a/qt-models/models.cpp b/qt-models/models.cpp
index ab80f95f0..7be9c2440 100644
--- a/qt-models/models.cpp
+++ b/qt-models/models.cpp
@@ -27,7 +27,7 @@ static QStringList getGasList()
 {
 	QStringList list;
 	for (int i = 0; i < displayed_dive.cylinders.nr; i++) {
-		const cylinder_t *cyl = &displayed_dive.cylinders.cylinders[i];
+		const cylinder_t *cyl = get_cylinder(&displayed_dive, i);
 		/* Check if we have the same gasmix two or more times
 		 * If yes return more verbose string */
 		int same_gas = same_gasmix_cylinder(cyl, i, &displayed_dive, true);