On Thu, Apr 28, 2022 at 10:34 AM Dirk Hohndel <d...@hohndel.org> wrote:
>
> > On Apr 28, 2022, at 10:30 AM, Linus Torvalds 
> > <torva...@linux-foundation.org> wrote:
> >
> > Can you send me the test-case so that I can follow along?
>
> Yep, will do off list.

Ok, so that makes *one* of the bugs very obvious.

The cylinder_with_sensor_sample() function indeed only tests "do we
have a mapping to this cylinder for this sample".

But it doesn't actually test "does the sample have an actual
_reading_?" as part of that.

That's what is causing the pointless "The deleted cylinder has sensor
readings, which will be lost." warning.

Trivial patch attached. We could do this other ways - maybe we could
do it as part of some cylinder cleanup phase where we just remove the
sensor[] thing for cylinders without any sensor data.

But this trivial patch is right regardless, and worth doing.

But I think one of the reports had a "subsurface crashes" in it too,
and I don't see _that_ part.

Was there some way to reproduce the crash? I ran it (very very slowly)
under valgrind too, and there were no complaints when removing the
cylinder (now without any warning), so it's at least not something
hugely obvious.

               Linus
 core/dive.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/core/dive.c b/core/dive.c
index 03eb82e70..7a4643571 100644
--- a/core/dive.c
+++ b/core/dive.c
@@ -3493,8 +3493,11 @@ extern bool cylinder_with_sensor_sample(const struct dive *dive, int cylinder_id
 {
 	for (const struct divecomputer *dc = &dive->dc; dc; dc = dc->next) {
 		for (int i = 0; i < dc->samples; ++i) {
+			struct sample *sample = dc->sample + i;
 			for (int j = 0; j < MAX_SENSORS; ++j) {
-				if (dc->sample[i].sensor[j] == cylinder_id)
+				if (!sample->pressure[j].mbar)
+					continue;
+				if (sample->sensor[j] == cylinder_id)
 					return true;
 			}
 		}
_______________________________________________
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to