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>
This case should be impossible to hit - but this seems better than
assuming this can never happen.
Coverity CID 1325458
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
When calling merge_two_dives both dives should be in the dive list so i
and j should never be -1. But just as extra precaution, bail if that's the
case (so that the helper functions below aren't called with negative
indices).
Coverity CID 1189514
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This seems a false positive, reply is always set before it is used, but
there's no harm in setting it explicitly.
Coverity CID 1325280
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
If we delete dives that were part of a trip, that trip may get deleted as
well. So if we undo that operation we need to bring back the trip, too.
This also deals with a bug in the original code that did the delete both
in calling code (in divelistview.cpp) and in the redo function. Because of
the nature of the delete this didn't really matter but it is of course
wrong and with the new code it would in fact cause an issue.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Right now this requires that
(a) the dive have only one divecomputer associated with it.
Trying to split a dive with multiple dive computers would be *much*
harder to do, since you'd have to try to line up the surface
interval between computers etc. So just don't do it after
downloading multiple dive computers for the same dive.
(b) there must be at least one minute between the sample that came up
to the surface and the sample that goes down again.
If you just peeked your head above the surface, don't try to split
things into two dives. Maybe we can relax this for freediving or
something.
also note that the split dive will only get new numbering if the dive
that was split was the very last dive in the divelist.
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 makes the code more robust in case the Uemis returns random or
non-sensical data. It's unlikely the user has a billion dives or that the
Uemis returns such a number. That's no reason not to handle this case
without crashing.
Coverity CID 1325289
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
I actually think this might be a false positive, but the libgit2 API
doesn't appear to guarantee that ancestor is not NULL, so let's add that
check.
Coverity CID 1325296
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This looks like possibly a false positive in the Coverity scan, but we can
always assume that the first point of the dive plan has been entered by
the user.
Coverity CID 1325285
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
It was ugly to show trip and dive location when no dive
was selected.
Signed-off-by: Tomaz Canabrava <tomaz.canabrava@intel.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This is different from a dive site, as it's not a dive site. It's just a
normal string, while a dive site has gps coordinates.
Signed-off-by: Tomaz Canabrava <tomaz.canabrava@intel.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This patch fixes an invalid dive site selection when
you where typing the name of a dive site for your current
dive.
Signed-off-by: Tomaz Canabrava <tomaz.canabrava@intel.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This may or may not look intuitive, but it can cause problems with the
zoom seemingly stuck all out (because of the timeouts). So instead stay
where you are. If the current dive site has GPS then its flag will be
bigger and brighter - so there still is visual feedback. But there's less
crazy zooming around.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Now it correctly sets the same dive site instead of
creating a new one for each dive.
Signed-off-by: Tomaz Canabrava <tomaz.canabrava@intel.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
But it will actually create a new dive site, not just rename the existing
one.
Signed-off-by: Tomaz Canabrava <tomaz.canabrava@intel.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
First make sure all the data in displayed dive is correctly recorded,
otherwise things could get overwritten when the filter is removed and we
redisplay the current dive.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
If the user added a dive manually we have a hard time trusting the start
time and duration. We just shouldn't automatically merge them with
anything else.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
If the user downloaded the GPS data from the Subsurface webservice before
naming a dive site, we run into a special case where entering a new name
for a dive location should just update the name of the automatically named
site which already has the correct GPS information.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This adds the QIODevice::Unbuffered flag to our rfcomm connections to
bypass the buffering layer in QIODevice.
This fixes so firmware upgrades work against the OSTC Sport.
Signed-off-by: Anton Lundin <glance@acc.umu.se>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
waitForReadyRead and waitForBytesWritten not overridden in
QBluetoothSocket and the default implementation in QIODevice are just
no-ops that always return false.
This removes those calls to lessen the confusion for anyone who looks at
the code.
Signed-off-by: Anton Lundin <glance@acc.umu.se>
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>
Normally the auto-created sites that are used when downloading GPS data
from the webservice that aren't associated with a dive will immediately
discarded. But if such a site is used for a dive but then the user changes
their mind and uses another site for that dive (for example because they
had a previous dive at that spot and decided they liked that one better
after all), then the dive site can stick around as an orphan.
Having orphan dive site may make sense for named dive sites that for some
reason are no longer used but may be used again in the future, but having
an auto-named orphaned dive site in the data file seems silly. So let's
remove them.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
In verbose mode we wanted to tell the user if they had a reference to a
non-existant dive site that we removed for consistency. Except the
condition was wrong and the warning was printed for every VALID dive site
reference. Oops.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
These messages are extremely useful to understand what Subsurface is
doing, but they are a bit too wordy for normal use.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
When you have openend cloud storage, the Save and Save to cloud storage
are basically the same thing... so we need to show the progress bar in
that case, too.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Under certain conditions on my Mac the build process will happily find libssh2
but then at link time can't figure out where it is. Making the path to the
library explicit seems to fix that issue.
This commit also adds the code to dump all visible variables at the end of the
cmake file (commented out). I have looked this up online, added it to address
an issue, and then removed it quite a few times... I figure I might as well
keep it there for the next time I need it.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
