scary changes...

Dirk Hohndel dirk at hohndel.org
Thu Nov 14 02:46:38 UTC 2013


Here's the commit message of the last commit I just pushed:

commit ed41d5a74490b9af03ece57015d6867bbf00df14
Author: Dirk Hohndel <dirk at hohndel.org>
Date:   Thu Nov 14 19:36:41 2013 +0900

    Sort the dive table after adding a dive
    
    This commit scares me. Any pointers to dives or indices into the dive
    table... anything like that is invalid after we resort the table. Well,
    not technically "invalid" (as in bad pointers), but after re-sorting the
    table these will possibly not be pointing at what we expected.
    
    This starts with the selection being "wrong" after we add a dive that
    isn't the last dive (once we click OK the chronologically last dive will
    be selected).
    
    But of course without doing this, our #1 assumption about the dive_table
    is broken. The dive_table is supposed to be in chronological order.
    
    Best advice of course would be "don't enter dives out of order" - but of
    course that's not realistic.
    
    Fixes #234
    
    Signed-off-by: Dirk Hohndel <dirk at hohndel.org>


I am reasonably certain that this is the right thing to do, but as the
commit message indicates, I'm rather worried about the consequences.
I have stared at this code for too long and would really appreciate some
others (especially Linus...) to take a look.

We do make the assumption that our dive_table is sorted in various
places (that's how the bug report #234 pointed me to this... if you add
dives out of chronological order you get negative time intervals between
dives... oops). But sadly we also have the habit of keeping either dive
indices (things like selected_dive) or (far worse) pointers to dives
around in memory. We should NEVER do the latter (except for time periods
where we KNOW that the dive_table won't be modified). The former
is /usually/ ok, but will still end up causing trouble if we for example
merge two dives or (as in this case) add dives in the middle of the
dive_table.

Please test.

/D



More information about the subsurface mailing list