From cfc1a68b0d01b4e61a2c828a2a8149ee3c5cbfb1 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Sun, 12 Jul 2015 21:19:28 +0200 Subject: [PATCH] Handle invalid logbook pointers as a fatal error. Until now, an invalid logbook pointer was silently ignored and handled as an empty ringbuffer. But this hides real errors, which is worse than failing if no dives are present. Trying to download dives from an empty device should be a rather uncommon scenario anyway. --- src/oceanic_common.c | 48 +++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/src/oceanic_common.c b/src/oceanic_common.c index f29d176..e8dbba4 100644 --- a/src/oceanic_common.c +++ b/src/oceanic_common.c @@ -246,39 +246,33 @@ oceanic_common_device_foreach (dc_device_t *abstract, dc_dive_callback_t callbac // Get the logbook pointers. unsigned int rb_logbook_first = array_uint16_le (pointers + 4); unsigned int rb_logbook_last = array_uint16_le (pointers + 6); - - // Convert the first/last pointers to begin/end/count pointers. - unsigned int rb_logbook_entry_begin, rb_logbook_entry_end, - rb_logbook_entry_size; if (rb_logbook_first < layout->rb_logbook_begin || rb_logbook_first >= layout->rb_logbook_end || rb_logbook_last < layout->rb_logbook_begin || rb_logbook_last >= layout->rb_logbook_end) { - // One of the pointers is outside the valid ringbuffer area. - // Because some devices use invalid pointers to indicate an - // empty ringbuffer, we silently ignore the error and always - // consider the ringbuffer empty. - rb_logbook_entry_begin = layout->rb_logbook_begin; - rb_logbook_entry_end = layout->rb_logbook_begin; - rb_logbook_entry_size = 0; + ERROR (abstract->context, "Invalid logbook pointer detected."); + return DC_STATUS_DATAFORMAT; + } + + // Convert the first/last pointers to begin/end/count pointers. + unsigned int rb_logbook_entry_begin, rb_logbook_entry_end, + rb_logbook_entry_size; + if (layout->pt_mode_global == 0) { + rb_logbook_entry_begin = rb_logbook_first; + rb_logbook_entry_end = RB_LOGBOOK_INCR (rb_logbook_last, layout->rb_logbook_entry_size, layout); + rb_logbook_entry_size = RB_LOGBOOK_DISTANCE (rb_logbook_first, rb_logbook_last, layout) + layout->rb_logbook_entry_size; } else { - if (layout->pt_mode_global == 0) { - rb_logbook_entry_begin = rb_logbook_first; - rb_logbook_entry_end = RB_LOGBOOK_INCR (rb_logbook_last, layout->rb_logbook_entry_size, layout); - rb_logbook_entry_size = RB_LOGBOOK_DISTANCE (rb_logbook_first, rb_logbook_last, layout) + layout->rb_logbook_entry_size; - } else { - rb_logbook_entry_begin = rb_logbook_first; - rb_logbook_entry_end = rb_logbook_last; - rb_logbook_entry_size = RB_LOGBOOK_DISTANCE (rb_logbook_first, rb_logbook_last, layout); - // In a typical ringbuffer implementation with only two begin/end - // pointers, there is no distinction possible between an empty and - // a full ringbuffer. We always consider the ringbuffer full in - // that case, because an empty ringbuffer can be detected by - // inspecting the logbook entries once they are downloaded. - if (rb_logbook_first == rb_logbook_last) - rb_logbook_entry_size = layout->rb_logbook_end - layout->rb_logbook_begin; - } + rb_logbook_entry_begin = rb_logbook_first; + rb_logbook_entry_end = rb_logbook_last; + rb_logbook_entry_size = RB_LOGBOOK_DISTANCE (rb_logbook_first, rb_logbook_last, layout); + // In a typical ringbuffer implementation with only two begin/end + // pointers, there is no distinction possible between an empty and + // a full ringbuffer. We always consider the ringbuffer full in + // that case, because an empty ringbuffer can be detected by + // inspecting the logbook entries once they are downloaded. + if (rb_logbook_first == rb_logbook_last) + rb_logbook_entry_size = layout->rb_logbook_end - layout->rb_logbook_begin; } // Check whether the ringbuffer is full.