Both gtk_tree_selection_selected_foreach() and
gtk_tree_selection_get_selected_rows() are problematic.
gtk_tree_selection_get_selected_rows is not compatible with older GTK,
while gtk_tree_selection_selected_foreach() should not be used to
modify the tree. A workaround to is allocate memory and store what
is returned from the gtk_tree_selection_selected_foreach() callback
function as a GtkTreeIter array. Once done iterate trought the array
and pass the values to delete_single_dive().
A bit excesive, but it is not certain how safe is modifying the tree
while in the "_foreach" loop, even if it only shows a warning.
On the other hand the GTK source shows gtk_tree_selection_get_selected_rows()
to be a rather complicated and slow method.
Inside delete_single_dive(), once a dive is no longer part of "dive_table"
and if the dive was part of a trip, remove the dive from the tree
(gtk_tree_store_remove()) and call update_trip_timestamp().
The struct type "tree_selected_st" and tree_selected_foreach() are
reusable.
Reported-by: Dirk Hohndel <dirk@hohndel.org>
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
gtk-gui.c:about_dialog():
Before assigning a pixel buffer to "logo" check if "image"
has a GTK_IMAGE_PIXBUF storage type.
Prevents the following message if the icon file is missing:
Gtk-CRITICAL **: gtk_image_get_pixbuf: assertion
The message that the file is missing is still shown.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
When calling gtk-gui.c:set_default_dive_computer_device(), first
free memory at default_dive_computer_device and then use strdup().
This fixed a case, where after selecting a device for the first time
in the "Download" dialog, then pressing "OK" followed by "Cancel"
(after a device error) the device text is garbled the next time the
dialog is opened.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Moved portion of the code from delete_dive_cb() to a function called
delete_single_dive(), that directly accepts a GtkTreeIter pointer.
Added the function delete_selected_dives_cb(), which is called
when calling "Delete dives" from the combo box for the selected dives.
The above function iterates trought the selection calling
delete_selected_foreach(), which on its own calls delete_single_dive().
The "for-each" API in this case looks much prettier C code wise,
however we do potentially create an extra jump and also
do not have anything but the redirection:
delete_selected_foreach() -> delete_single_dive()
Probably slighly slower than using gtk_tree_selection_get_selected_rows(),
performance wise, but less C code.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
When clicking multiple dives in the list, check if more than one
are selected and if so show the text "Delete dives".
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
If we have no explicit cylinder info at all (it's normal air, no size or
working pressure information, and no beginning/end pressure information),
we don't save the cylinders in question because that would be redundant.
Such non-saved cylinders may still show up in the equipment list because
there may be implicit mention of them elsewhere, notably due to sample
data, so not saving them is the right thing to do - there is nothing to
save.
However, we missed one case: if there were other cylinders that *did* have
explicit information in it following such an uninteresting cylinder, we do
need to save the cylinder information for the useless case - if only in
order to be able to save the non-useless information for subsequent
cylinders.
This patch does that. Now, if you had an air-filled cylinder with no
information as your first cylinder, and a 51% nitrox as your second one,
it will save that information as
<cylinder />
<cylinder o2='51.0%' />
rather than dropping the cylinder information entirely.
This bug has been there for a long time, and was hidden by the fact that
normally you'd fill in cylinder descriptions etc after importing new
dives. It also used to be that we saved the cylinder beginning/end
pressure even if that was generated from the sample data, so if you
imported from a air-integrated computer and had samples for that cylinder,
we used to save it even though it was technically redundant.
We stopped saving redundant air sample information in commit 0089dd8819
("Don't save cylinder start/end pressures unless set by hand").
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Removed start and end in save_cylinder_info(). These two variables are no
longer used.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Renamed the function pick_import_files() to import_files()
and added a declaration in "display-gtk.h". The declaration is "extern"
for consistency with download_dialog().
Since the new dialog is a real child off "main_window", we no longer need
some of the calls to gtk_widget_set_sensitive(), gtk_window_set_decorated()
to disable the parent, which previously was the old "Import" combined dialog.
Once the file list is retrieved, attempt to import each file and update
the list via report_dives().
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Minor changes to file select box title
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Renamed "import_dialog" to "download_dialog" and also focused the download
dialog strictly on dive computer functionality. Removed the file button
and "filenames" checks from "download_dialog".
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
New entries with icons and shortcut keys in the menus.
"Import XML File" is now using CTRL + I.
Reserve CTRL + E for "Export XML File" if possible.
"Download From Dive Computer" is using CTRL + D and
a GTK_STOCK_GO_DOWN icon.
Added separator below this entry in the "Log" menu.
Note: menu entry callbacks are NULL at this stage.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
There are possible bugs when targeting GTK 2.24.10.
When opening the "import XML files" file selector from the "import" dialog
on Ubuntu 12.04, even with gtk_window_set_accept_focus() setting FALSE
to the parent ("import"), it is still possible to click the title bar
and buttons, causing actions. This can trigger undefined behaviour -
e.g. opening multiple "import XML files" dialogs.
While on Windows the situation is more acceptable, on Ubuntu it is also
possible to click the [x] button on the title bar of the parent window,
which will close the parent dialog _with_ the current forward dialog.
Multiple proposed methods in the GTK documentation seem not to function
as expected, such as: gtk_window_set_modal() and gtk_window_set_accept_focus().
Passing the correct parent on dialog creation doesn't seems to help either.
The same problem occurs when selecting a default file from the preferences
dialog.
The proposed portable solution in this patch is to completely disable
the background windows by hiding the titlebar (gtk_window_set_decorated),
disabling child controls (gtk_widget_set_sensitive) and making the window
"transient for" or putting it on top (gtk_window_set_transient_for).
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Renamed the XML button and titles slightly.
Generalized the dialog topmost text to "Choose what to import:".
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
After deleting a dive the dive list is recreated. If there are still dives
selected we should select the last dive as well. If there isn't any dive
selected, then the last dive is as good a default as any, I guess.
Reported-by: "Lubomir I. Ivanov" <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
When switching the dive trips to be stored in a different data structure I
forgot to update the code in save_trip() - and since we were passing the
pointer around via a gpointer the compiler didn't catch this. Oops.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
There's no guarantee that gtk is thread-safe (apparently it can be
broken at least on Windows, even though it should be fine on top of X).
So don't update the progress bar directly from the dive computer import
code, instead just update the progress information in static variables,
and let the GUI thread update it while it does the idle loop polling
anyway.
Reported-by: Jef Driesen <jefdriesen@telenet.be>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
One of the "+1900" cases got incorrectly dropped.
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
[ I really wanted the utc_mkdate/mktime functions to use proper years,
rather than the idiotic "offset by 1900". However, we still mix
things with "localtime()" in one place, so for now we have to be
compatible with the insane tm_year semantics. - Linus ]
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Merge the dive trip rewrite by Dirk Hohndel.
This just merges the dive trip changes with the timestamp handling
changes. There were multiple small data conflicts, along with some
newly added 'time_t' cases in the dive trip handling that needed to be
converted to 'timestamp_t' along the way.
* 'divetrip-rewrite' of git://github.com/torvalds/subsurface:
Convert FIND_TRIP into function
Partial rewrite of the dive trip code
Check if trip is NULL before calling DIVE_TRIP
Merge the 64-bit timestamp_t time function branch.
This makes subsurface not only safe against the 2038-year problem, but
also avoids the use of thread-unsafe gmtime() etc.
We still use the system time_t for initializing the calendar widget for
adding a new dive, but that's cosmetic rather than anything fundamental.
* time-function:
FIND_TRIP: don't cast a timestamp to a pointer
dive-time widget: fix incorrect use of timestamp_t
Fix the incorrect data type for DIVE_DATE accesses
Use a 64-bit 'timestamp_t' for all timestamps, rather than 'time_t'
The pointer size may not be large enough to contain a timestamp, so make
FIND_TRIP() just pass the pointer to the timestamp instead.
And use an inline function instead of macros with casts. That gets us
proper type safety while at it, so that we get a warning if somebody
doesn't pass the expected "timestamp_t *". Plus the code actually looks
simpler and way more straightforward.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
I did a global search-and-replace to make all "time_t" users use the
internal subsurface 64-bit "timestamp_t" type instead, but we have one
case that still uses the system time functions: the use of "localtime()"
in the dive_time_widget().
Everywhere else we always just use UTC for all our time handling, and we
don't really ever care about the local timezone etc. However, for the
dive time widget, we initialize the calendar widget to the current time,
which obviously does want to take the local timezone into account, so
there we end up using the whole system time handling code.
So that one should continue to use time_t, even if it might have the
year-2038 problem. We also don't care about the fact that it's not
thread-safe, since this is just initializing the widget which definitely
doesn't happen threaded.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This helps us deal with the issue that the g_list convenience functions
don't allow us to easily compare 64bit values on 32bit architectures. And
since these convenience functions are truly trivial in nature, it seemed
easier to simply implement our own logic here.
In the process I moved all the dive_trip_list helper functions into the
same spot in divelist.c
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This is the same bugfix that Lubomir did in the master branch, but now
on top of the new 64-bit timestamp_t model. So now we also remove the
comment about the year 2038 problem, because it's not true any more. We
do all the date handling in a 64-bit integer.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This introduces a new data structure for dive trips - reuseing the struct
dive just got way too messy.
The dive_trip_t datastructure now allows the code to remember if the trip
was auto generated or if its time stamp changed when dives where added to
the trip during auto generation.
The algorithm also distinguishes between dives that were intentionally
added to a trip (either in an XML file or by adding them to trip in the
UI) and dives that were added to trips via autogen. Saving dives that were
added to trips via autogen makes that assignment "intentional".
With this partial rewrite several of the oddities of the old code should
be resolved - especially turning autogen on and off again should get the
divelist back to the previous stage.
Also, when dives are merged during file open or import we now try to pick
the correct tripflag (instead of just ignoring the tripflag completely and
resetting it to TF_NONE by mistake).
Finally, the dive trip debugging code got more verbose and is trying
harder to detect issues at the earliest time possible.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The type of the DIVE_DATE field is G_TYPE_LONG, so when accessing it we
have to pass gtk_tree_model_get() the proper "long *" pointer, rather
than an int. On 64-bit platforms, passing a pointer to an int would
otherwise result in randomly overwriting another four bytes of stack.
Sadly, there are not much safety checks to warn if our passed variable
to the vararg list is of the correct type, which is unlike
gtk_tree_model_get_value(), which will report typed errors.
This solves a bug that may potentially result in undefined behaviour
on some x64 OS (e.g Ubuntu 12.04 x64).
Reported-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This makes the time type unambiguous, and we can use G_TYPE_INT64 for it
in the divelist too.
It also implements a portable (and thread-safe) "utc_mkdate()" function
that acts kind of like gmtime_r(), but using the 64-bit timestamp_t. It
matches our original "utc_mktime()".
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Other places have this check, but for this particular one a crash
can be reproduced:
./subsurface ./dives/*
log -> autogroup
log -> autogroup
Against d14932058f
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Thanks to Christian for running the static code analysis tool against
subsurface...
There were some false positives, a few style issues that I'll ignore for
now, and two actual potential bugs.
First: Don't check unsigned variables for < 0
This has been around for a while and we are lucky that while technically a
bug it still works as expected. Passing a negative idx simply turns it
into a very large unsigned integer which then fails the > dive_table.nr
test. So it still gets a NULL returned. A bug? Yes. Critical? No.
Mismatched allocation and free
This is an actual bug that potentially could cause issues. We allocate
memory with malloc and free it with g_free. Not good.
Reported-by: Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn@axis.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This could cause a crash if deleting the last dive and manually adding a
new one.
Reported-by: Henrik Brautaset Aronsen <subsurface@henrik.synth.no>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
After Lubomir's latest changes to the File menu, the separator
were a little off.
Signed-off-by: Henrik Brautaset Aronsen <subsurface@henrik.synth.no>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This fixes a bug that Lubomir reported in a different way from the patch
that he providede; I believe this to be more generic.
Reported-by: "Lubomir I. Ivanov" <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Removed redundant /oname settings when copying files. This is not required
since the file name is not changed.
Signed-off-by: Ivan Habunek <ivan.habunek@gmail.com>
Windows Vista and later require admin privileges to install to the Program
Files folder. Updated RequestExecutionLevel accordingly.
Signed-off-by: Ivan Habunek <ivan.habunek@gmail.com>
Related to subsurface_default_filename() and g_path_get_basename().
Against 3835faa8fb.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Currently doubles the functionality of "Close" (file_close).
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Since the GSList is now only created if the user enters the file selection
dialog, opening the import dialog and then clicking OK without selecting
either a dive computer or a file would cause a crash.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Renamed the title of the "Import" dialog to "Import", as there
are a couple of supported operations.
Removed the "Import:" text in the dialog body.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Moved the entry bellow the standard file operations and above "Print".
Also placed it between separators and added the GTK_STOCK_GO_BACK icon.
Later on "Export" can be placed below "Import" using GTK_STOCK_GO_FORWARD.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
By now the default file code seems quite matured, so in preparation for
2.0 we'll bring it back into master.
I made a few small clean-ups during the merge, but the merge itself is
very much straight forward.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Commit cdae2869d1dd ("Show the datafile name even with no dives") was a
little too aggressive in making sure that we show the correct window title
- we only should call show_dive_info(NULL) if there is indeed no dive in
the dive table - otherwise we display empty dive info at program start.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This doesn't change functionality - it's just pointless to loop over a
list that is known to have only one element.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This logic seems to make much more sense - if the user hits 'OK' then the
old file is closed and the new one openened. Otherwise, leave things
unchanged.
Reported-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Several potential problems.
- we could end up dereferencing exiting_filename when it was NULL
- we could free the default_filename by mistake -
subsurface_default_filename always needs to return a copy of it
- closing the existing file before opening a new one repopulated the
existing_filename with the default filename - preventing the opened
file to become the new existing filename
Also, make existing filename a const char * and make file_open have the
same sensible default folder behavior as the other file related functions.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
If the last of the preexisting dives gets merged with a new dive we end up
dereferencing a freed pointer.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Previously we could end up with a bogus dive with all zero data in it.
Adding dives/test24.xml to be able to test that we handle this case
correctly.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
That's especially useful if starting without a filename and without an
existing default file - this way it's clear that Subsurface still
considers itself synced with the default file.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
