So Anton Lundin says that the 32-bit timestamp for the Divesoft Freedom
is indeed a signed offset from Jan 1, 2000.
This does that, but also extracts the whole thing into a helper function
and makes sure that there are no overflows at any point by using
"timestamp_t" in the whole series, and all the operations are "obviously
safe" in their types (ie no "unsigned char gets widened to 'int' and
then we shift it left by 24 bits").
Signed-off-by: Linus Torvalds <torvalds@linux-fundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Commit 31fb2e4c62 ("Avoid possible sign extension") handled the
problem when a "unsigned char" is shifted 24 bits left, and becomes a
"signed int". By casting the result to uint32_t, that signed case won't
happen.
However, there were two bugs in that fix.
The first is the comment. It's not that "timestamp_t" is signed that is
the problem. No, the problem is inherent in the C expression
(ptr[11] << 24)
where "ptr[11]" is an unsigned char. In C arithmetic, unsigned char is
implicitly type-expanded to "int", so while it has a value between
0..255, when you shift it left by 24, you can get a *negative* "int" as
a result.
So it's actually "ptr[11]" that should have been cast to "unsigned", but
it so happens that you can do all the shifting and adding in "int", and
then cast the end result to "uint32_t" and you'll get the same value.
But at no point did "timestamp_t" matter.
The other bug was pre-existing and just not fixed. When the code does
the "+ 946684800" (to turn the timestamp to be seconds from the start of
2000, into seconds since the "unix epoch", ie 1970) that arithmetic is
now done in that "uint32_t" (and used to be done in "int").
Which means that the addition can overflow in 32 bits *before* it is
cast to timestamp_t (which is 64 bits).
Admittedly that 32-bit overflow happens a bit later than the sign bit
gets set, but if we're worried aboout overflows, let's just do this
right.
In other words, we have a 32-bit unsigned offset since Jan 1, 2000, and
for the full range we need to do the epoch correction in 32 bits.
Because otherwise you fail in the year 2106 (32-bit unsigned unix epoch
time limit), even though the 32-bit seconds *should* work all the way
until the year 2136.
Of course, I'll be rather surprised if people still use the Divesoft
Freedom in the year 2106. Or rather, I won't be surprised, because I'll
be dead.
But if we think that the signed problem matters (in the year 2068), then
dammit, we can extend it another 30 years.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We only made sure the divelist allocation was big enough in
"record_dive_to_table()", but add_single_dive() can add entries too.
Now, in practice that never bit anybody, since
(a) we allocate extra entries anyway, and it would be very unusual that
the divelist table was exactly full
(b) most "malloc()" implementations end up having their own slop on top
of that
(c) add_single_dive() was only used for merging dives, which actually
ends up removing more divex than it adds (but it does add one first)
but when I'm starting to split dives, this will be a bigger issue in
practice. And it was wrong.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This shouldn't happen, but in case there is a logic error higher up in the
code somewhere, this will prevent it from happening, period.
If the code asks for a new dive site with a specific uuid, simply return
the existing dive site with that uuid.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This teaches subsurface how to understand more fields in the xml that
libdivecomputers's universal tool generates.
Signed-off-by: Anton Lundin <glance@acc.umu.se>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Assume your V2 file contains two locations with different name but
identical GPS location. "Blue Corner" and "Blue Corner, Palau". And you
have many dives there.
When reading a V2 file the GPS is read first and a dive site is looked up
based on that. Let's assume the lookup by gps finds "Blue Corner, Palau".
Now every time we get the "Blue Corner" site in the V2 file we look up
the GPS, get "Blue Corner, Palau" as dive site, then read the name "Blue
Corner" and say "oops, different site" and create a new one. Resulting in
several dive sites named "Blue Corner" with identical GPS but different
UUID (as we add the dive time into the SHA for the deterministic UUID).
With this commit, if we have a dive site that matches the GPS but has a
different name, we check if we happen to have an exact match for the dive
site information from the XML file (and use it, if we have it) before
creating a new dive site.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Having random uuids seemed like a good idea, but there are several
situations where they really cause problems. One is merging dive file
imports from V2 logfiles. Another is testing such imports.
Instead of making the uuid random we now hash the name and add the
timestamp of the first dive associated with this dive site to the hash
(first in this context is "first encountered" with no guarantee that it is
the chronologically first). This way V2 imports create deterministic uuids
but uuid conflicts are still extremely unlikely, even if the user has
multiple dive sites with the same name.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Let's do the bit shift properly and not by block size.
Fixes#906
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Subsurface supports currently 8 cylinders, thus we should not attempt to
add more.
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Test cases require deterministic results and thus we should allow uuid
to be specified when needed.
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We obviously should lookup if the dive site already exist before
creating a new one.
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Max depth is recorded in floating point in metadata.
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This is my best guess, and I have no sample logs that would prove my
assumption.
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This shows the ceiling as stored by Divinglog. (I am not sure if it is
DC reported ceiling, or just a calculated one.)
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Note, that this patch is not tested with real data as I do not have log
that would include heartbeat available.
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The way we freed things and cleared out the variables potentially left
dangling data behind and could end up calling free on garbage data,
leading to random crashes.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Cylinder 0 is stored on the Logbook table along with other dive metadata
(not in Tank table).
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Since mapping cylinders requires use of dive ID, we might as well use
the ID to map the profile also.
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This parses the basic metadata of a dive when importing Divinglog
database. (Discarding deleted dives is my best guess as I do not have
that in my sample dive.)
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Remove several very noise messages on dive site handling (this seems to
work well now, so I think we can remove most of them - a couple were left
that indicate actual issues).
And also remove all the calls to "translate" when outputting data to
stderr. Error messages that indicate issues where the user will basically
have to come and ask the developers for help shouldn't be localized. They
should be in English to make it easier for us to figure out what's going
on.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
If taxonomy data are available we are switching a dive site entry from
single item with attributes to an item that has children. This is
backwards compatible and older versions will simply ignore the children.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This might be what the user wanted, but also might not
be it. So wait for the user to request it manually.
Signed-off-by: Tomaz Canabrava <tomaz.canabrava@intel.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Free memory returned from parse_mkvi_value()
Free memory returned from printGPSCoords()
Free memory allocated in added_list and removed_list
Free memory allocated when adding suffix to dive site name
Free memory allocated in cache_deco_state()
Free memory allocated in build_filename()
Free memory allocated in get_utf8()
Free memory allocated in alloc_dive()
Free memory allocated as cache but never used
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Add infrastructure and helper functions to track minimum datafile version.
To make this information useful we need to keep the XML and git data
format versions in track moving forward.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This was a poorly implemented hack when we executed the reverse geo lookup
in the main thread and opening a V2 file could take a very long time. We
need to do the "Welcome" message quite differently.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
