[PATCH 2/2] Ticket #831 Fix

[Dirk Hohndel]

On Tue, Mar 10, 2015 at 12:52:16AM +0200, Yosef Hamza wrote:
> Here's the proposed change by Robert fixes the problem for all.

Well, it may fix one problem, but using that patch would create a lot of
others...

This whole "two patch rule" is about making sure you understand how to
submit changes and how to deal with feedback... so here we go :-)

>From ac8291f5f71e4bf17a80f0e82758066174808c9c Mon Sep 17 00:00:00 2001
From: Yousef Hamza <jo.adam.93 at gmail.com>
Date: Tue, 10 Mar 2015 00:44:46 +0200
Subject: [PATCH] Preventing more than one events @0:00

Removing existing gas change events @0:00 when
new one is added.

Signed-off-by: Yousef Hamza <jo.adam.93 at gmail.com>
---
 qt-ui/profile/profilewidget2.cpp | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/qt-ui/profile/profilewidget2.cpp b/qt-ui/profile/profilewidget2.cpp
index cfcd25d..ab59330 100644
--- a/qt-ui/profile/profilewidget2.cpp
+++ b/qt-ui/profile/profilewidget2.cpp
@@ -1382,6 +1382,9 @@ void ProfileWidget2::changeGas()
 	// backup the things on the dataModel, since we will clear that out.
 	struct gasmix gasmix;
 	int seconds = timeAxis->valueAt(scenePos);
+	if (seconds < 0) {
+		seconds = 0;
+	}

So this SHOULD be its own commit. I simply disagreed with your commit
message. Please submit as a separate change with an appropriate message.

 
@@ -1392,6 +1395,23 @@ void ProfileWidget2::changeGas()
 		qDebug() << "failed to parse tank number";
 		tank = get_gasidx(&displayed_dive, &gasmix);
 	}
+
+	struct event **events = &get_dive_dc(&displayed_dive, dc_number)->events;
+	while (*events) {
+		if ((*events)->time.seconds == 0) {
+			remove_event(*events);

You cannot use remove_event() on events from displayed_dive. It only
removes events from current_dive

+		}
+		events = &(*events)->next;

so you remove *events and THEN use *events->next ?
That's called accessing freed memory...

+	}
+
+	events = &current_dc->events;
+	while (*events) {
+		if ((*events)->time.seconds == 0) {
+			remove_event(*events);
+		}
+		events = &(*events)->next;

Same as abive.

But regardless of the issues oiubted iytm this whole bit of code is flat
out wrong. You unconditionally delete ALL events at time stamp zero. And
that is not what you should be doing here.

What you should do is that IFF the user adds a gas change at time 0 and
IFF there already is a gas change at time 0, THEN remove that existing gas
change.

Look around the sources... there's a helper function that you can use to
search for the next gas change event. Use that on the events pointer and
check if it returns a gas change at time 0...

/D