[PATCH] Crash fix in add_single_dive. No writing to dive_table.dives[-1]

K. "pestophagous" Heller pestophagous at gmail.com
Wed Nov 25 10:29:02 PST 2015


On Wed, Nov 25, 2015 at 10:16 AM, Lubomir I. Ivanov <neolit123 at gmail.com> wrote:
> On 25 November 2015 at 20:01, K. "pestophagous" Heller
> <pestophagous at gmail.com> wrote:
>> On Wed, Nov 25, 2015 at 6:53 AM, Lubomir I. Ivanov <neolit123 at gmail.com> wrote:
>>>
>>> currently add_single_dive() assumes a safe index.
>>> i think that add_single_dive() should not be touched, but instead the
>>> mobile app should be fixed (models bug?).
>>>
>>
>> That sounds reasonable, but i would mention the following:
>>
>> 1. if add_single_dive is going to assume a safe index, then we should
>> add an assert. an assertion failure would have been preferable to
>> varied and mysterious crashes after the call.
>>
>> 2. the ability to call with -1 is also a feature. currently, if you
>> want to add a dive to the *front* of the list, you pass 0.  But if you
>> want to add it to the *end* of the list, you need to know the size of
>> the table (so you can pass the table size as your targeted insertion
>> point).  Passing 0 to mean "prepend" does not require knowledge of the
>> current table size.  So, using -1 as a shorthand for "append" provides
>> a similar feature, then. no need to know the table size.
>>
>
> it's probably better to have the feature, over the assert().
> then the mobile app should make sure if it really wants to add to the
> back of the list with a negative value.
>
> lubomir
> --

cool. agreed. i do want to continue examining what the mobile app is
doing with 'Add Dive.' the add-dive mobile option needs work. but i
had to get past the crashing first :)

/K


More information about the subsurface mailing list