Update cylinder info properly

The "cylinders_equal()/copy_cylinders()" functions were buggered, and
only checked (and copied) the cylinder type.  That was on purpose, since
you do want to be able to change the type of a cylinder without changing
the gasmix of the cylinder.

HOWEVER, the reverse is also true: you may want to change the gasmix of
a cylinder without changing the type.

So it's not that the type of the cylinder is special - it's that the
type and the gasmix should be considered separately.

Do that properly for the equipment editing case.

Reported-by: Ďoďo <dodo.sk@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

3 files changed, 81 insertions, 40 deletions

diff --git a/dive.h b/dive.h
index e202279af..4b00b4eba 100644
--- a/dive.h
+++ b/dive.h
@@ -97,8 +97,6 @@ extern gboolean cylinder_nodata(cylinder_t *cyl);
 extern gboolean cylinder_nosamples(cylinder_t *cyl);
 extern gboolean cylinder_none(void *_data);
 extern gboolean no_cylinders(cylinder_t *cyl);
-extern gboolean cylinders_equal(cylinder_t *cyl1, cylinder_t *cyl2);
-extern void copy_cylinders(cylinder_t *cyl1, cylinder_t *cyl2);
 extern gboolean no_weightsystems(weightsystem_t *ws);
 extern gboolean weightsystems_equal(weightsystem_t *ws1, weightsystem_t *ws2);
 
diff --git a/equipment.c b/equipment.c
index 7c473f6df..ccdee6268 100644
--- a/equipment.c
+++ b/equipment.c
@@ -485,40 +485,6 @@ gboolean description_equal(const char *desc1, const char *desc2)
 			(desc1 && desc2 && strcmp(desc1, desc2) == 0));
 }
 
-/* when checking for the same cylinder we want the size and description to match
-   but don't compare the start and end pressures, nor the Nitrox/He values */
-static gboolean one_cylinder_equal(cylinder_t *cyl1, cylinder_t *cyl2)
-{
-	return cyl1->type.size.mliter == cyl2->type.size.mliter &&
-		cyl1->type.workingpressure.mbar == cyl2->type.workingpressure.mbar &&
-		description_equal(cyl1->type.description, cyl2->type.description);
-}
-
-gboolean cylinders_equal(cylinder_t *cyl1, cylinder_t *cyl2)
-{
-	int i;
-
-	for (i = 0; i < MAX_CYLINDERS; i++)
-		if (!one_cylinder_equal(cyl1 + i, cyl2 + i))
-			return FALSE;
-	return TRUE;
-}
-
-/* copy size and description of all cylinders from cyl1 to cyl2 */
-void copy_cylinders(cylinder_t *cyl1, cylinder_t *cyl2)
-{
-	int i;
-
-	for (i = 0; i < MAX_CYLINDERS; i++) {
-		cyl2[i].type.size.mliter = cyl1[i].type.size.mliter;
-		cyl2[i].type.workingpressure.mbar = cyl1[i].type.workingpressure.mbar;
-		if (cyl1[i].type.description)
-			cyl2[i].type.description = strdup(cyl1[i].type.description);
-		else
-			cyl2[i].type.description = NULL;
-	}
-}
-
 static gboolean weightsystem_none(void *_data)
 {
 	weightsystem_t *ws = _data;
diff --git a/info.c b/info.c
index 96e270459..ce7240c39 100644
--- a/info.c
+++ b/info.c
@@ -519,18 +519,95 @@ static void save_equipment_data(struct dive *dive)
 	}
 }
 
+/* Empty and NULL compare equal */
+static int same_string(const char *a, const char *b)
+{
+	/* Both NULL or same */
+	if (a == b)
+		return 1;
+	/* Both non-NULL: strcmp */
+	if (a && b)
+		return !strcmp(a, b);
+	/* One non-NULL? Is that one empty? */
+	return !*(a ? a : b);
+}
+
+static int same_type(cylinder_t *dst, cylinder_t *src)
+{
+	return	dst->type.size.mliter == src->type.size.mliter &&
+		dst->type.workingpressure.mbar == src->type.workingpressure.mbar &&
+		same_string(dst->type.description, src->type.description);
+}
+
+static void copy_type(cylinder_t *dst, cylinder_t *src)
+{
+	dst->type.size = src->type.size;
+	dst->type.workingpressure = src->type.workingpressure;
+	if (dst->type.description)
+		free((void *)dst->type.description);
+	if (!src->type.description || !*src->type.description)
+		dst->type.description = NULL;
+	else
+		dst->type.description = strdup((char *)src->type.description);
+}
+
+static int same_gasmix(cylinder_t *dst, cylinder_t *src)
+{
+	return !memcmp(&dst->gasmix, &src->gasmix, sizeof(dst->gasmix));
+}
+
+static void copy_gasmix(cylinder_t *dst, cylinder_t *src)
+{
+	memcpy(&dst->gasmix, &src->gasmix, sizeof(dst->gasmix));
+}
+
+static int same_press(cylinder_t *dst, cylinder_t *src)
+{
+	return	dst->start.mbar == src->start.mbar &&
+		dst->end.mbar == src->end.mbar;
+}
+
+static void copy_press(cylinder_t *dst, cylinder_t *src)
+{
+	dst->start = src->start;
+	dst->end = src->end;
+}
+
+/*
+ * When we update the cylinder information, we do it individually
+ * by type/gasmix/pressure, so that you can change them separately.
+ *
+ * The rule is: the destination has to be the same as the original
+ * field, and the source has to have changed. If so, we change the
+ * destination field.
+ */
+static void update_cylinder(cylinder_t *dst, cylinder_t *src, cylinder_t *orig)
+{
+	/* Destination type same? Change it */
+	if (same_type(dst, orig) && !same_type(src, orig))
+		copy_type(dst, src);
+
+	/* Destination gasmix same? Change it */
+	if (same_gasmix(dst, orig) && !same_gasmix(src, orig))
+		copy_gasmix(dst, src);
+
+	/* Destination pressures the same? */
+	if (same_press(dst, orig) && !same_press(src, orig))
+		copy_press(dst, src);
+}
+
 /* the editing happens on the master dive; we copy the equipment
    data if it has changed in the master dive and the other dive
    either has no entries for the equipment or the same entries
    as the master dive had before it was edited */
 void update_equipment_data(struct dive *dive, struct dive *master)
 {
+	int i;
+
 	if (dive == master)
 		return;
-	if ( ! cylinders_equal(remember_cyl, master->cylinder) &&
-		(no_cylinders(dive->cylinder) ||
-			cylinders_equal(dive->cylinder, remember_cyl)))
-		copy_cylinders(master->cylinder, dive->cylinder);
+	for (i = 0; i < MAX_CYLINDERS; i++)
+		update_cylinder(dive->cylinder+i, master->cylinder+i, remember_cyl+i);
 	if (! weightsystems_equal(remember_ws, master->weightsystem) &&
 		(no_weightsystems(dive->weightsystem) ||
 			weightsystems_equal(dive->weightsystem, remember_ws)))