mifr/subsurface
1
0
Fork 0
mirror of https://github.com/subsurface/subsurface.git synced 2025-02-19 22:16:15 +00:00
Code Issues Projects Releases Packages Wiki Activity

fix copy/paste of dive-site

Browse source 
The copy/pasting of dive-sites was fundamentally broken in at least two
ways:

1) The dive-site pointer in struct dive was simply overwritten, which
   breaks internal consistency. Also, no dive-site changed signals where
   sent.

2) The copied dive-site was stored as a pointer in a struct dive. Thus,
   the user could copy a dive, then delete the dive-site and paste.
   This would lead to a dangling pointer and ultimately crash the
   application.

Fix this by storing the UUID of the dive-site, not a pointer.
To do that, don't store a copy of the dive, but collect all
the data in a `dive_paste_data` structure.
If the dive site has been deleted on paste, do nothing.
Send the appropriate signals on pasting.

The mobile version had an additional bug: It kept a pointer to the
dive to be copied, which might become stale by undo.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2024-08-13 07:04:52 +02:00 • committed by Michael Keller
parent 48b4308a7d
commit 152e6966c9
17 changed files with 359 additions and 425 deletions
Download patch file Download diff file Expand all files Collapse all files

4
commands/command.cpp
View file

@ -267,9 +267,9 @@ int editDiveGuide(const QStringList &newList, bool currentDiveOnly)
return execute_edit(new EditDiveGuide(newList, currentDiveOnly));
}
void pasteDives(const dive *d, dive_components what)
void pasteDives(const dive_paste_data &data)
{
execute(new PasteDives(d, what));
execute(new PasteDives(data));
}
void replanDive(dive *d)

3
commands/command.h
View file

@ -13,6 +13,7 @@
struct divecomputer;
struct divelog;
struct dive_components;
struct dive_paste_data;
struct dive_site;
struct dive_trip;
struct event;
@ -95,7 +96,7 @@ int editDiveSiteNew(const QString &newName, bool currentDiveOnly);
int editTags(const QStringList &newList, bool currentDiveOnly);
int editBuddies(const QStringList &newList, bool currentDiveOnly);
int editDiveGuide(const QStringList &newList, bool currentDiveOnly);
void pasteDives(const dive *d, dive_components what);
void pasteDives(const dive_paste_data &data);
enum class EditProfileType {
ADD,
REMOVE,

200
commands/command_edit.cpp
View file

@ -613,41 +613,51 @@ QString EditDiveGuide::fieldName() const
return Command::Base::tr("dive guide");
}
PasteState::PasteState(dive *dIn, const dive *data, dive_components what) : d(dIn)
template <typename T>
ptrdiff_t comp_ptr(T *v1, T *v2)
{
if (what.notes)
notes = data->notes;
if (what.diveguide)
diveguide = data->diveguide;
if (what.buddy)
buddy = data->buddy;
if (what.suit)
suit = data->suit;
if (what.rating)
rating = data->rating;
if (what.visibility)
visibility = data->visibility;
if (what.wavesize)
wavesize = data->wavesize;
if (what.current)
current = data->current;
if (what.surge)
surge = data->surge;
if (what.chill)
chill = data->chill;
if (what.divesite)
divesite = data->dive_site;
if (what.tags)
tags = data->tags;
if (what.cylinders) {
cylinders = data->cylinders;
return v1 - v2;
}
PasteState::PasteState(dive &d, const dive_paste_data &data, std::vector<dive_site *> &dive_sites_changed) : d(d)
{
notes = data.notes;
diveguide = data.diveguide;
buddy = data.buddy;
suit = data.suit;
rating = data.rating;
visibility = data.visibility;
wavesize = data.wavesize;
current = data.current;
surge = data.surge;
chill = data.chill;
if (data.divesite.has_value()) {
if (data.divesite) {
// In the undo system, we can turn the uuid into a pointer,
// because everything is serialized.
dive_site *ds = divelog.sites.get_by_uuid(*data.divesite);
if (ds)
divesite = ds;
} else {
divesite = nullptr;
}
}
if (divesite.has_value() && *divesite != d.dive_site) {
if (d.dive_site)
range_insert_sorted_unique(dive_sites_changed, d.dive_site, comp_ptr<dive_site>); // Use <=> once on C++20
if (*divesite)
range_insert_sorted_unique(dive_sites_changed, *divesite, comp_ptr<dive_site>); // Use <=> once on C++20
}
tags = data.tags;
if (data.cylinders.has_value()) {
cylinders = data.cylinders;
// Paste cylinders is "special":
// 1) For cylinders that exist in the destination dive we keep the gas-mix and pressures.
// 2) For cylinders that do not yet exist in the destination dive, we set the pressures to 0, i.e. unset.
// Moreover, for these we set the manually_added flag, because they weren't downloaded from a DC.
for (size_t i = 0; i < d->cylinders.size() && i < cylinders.size(); ++i) {
const cylinder_t &src = d->cylinders[i];
cylinder_t &dst = cylinders[i];
for (size_t i = 0; i < data.cylinders->size() && i < cylinders->size(); ++i) {
const cylinder_t &src = (*data.cylinders)[i];
cylinder_t &dst = (*cylinders)[i];
dst.gasmix = src.gasmix;
dst.start = src.start;
dst.end = src.end;
@ -661,8 +671,8 @@ PasteState::PasteState(dive *dIn, const dive *data, dive_components what) : d(dI
dst.bestmix_o2 = src.bestmix_o2;
dst.bestmix_he = src.bestmix_he;
}
for (size_t i = d->cylinders.size(); i < cylinders.size(); ++i) {
cylinder_t &cyl = cylinders[i];
for (size_t i = data.cylinders->size(); i < cylinders->size(); ++i) {
cylinder_t &cyl = (*cylinders)[i];
cyl.start.mbar = 0;
cyl.end.mbar = 0;
cyl.sample_start.mbar = 0;
@ -670,61 +680,62 @@ PasteState::PasteState(dive *dIn, const dive *data, dive_components what) : d(dI
cyl.manually_added = true;
}
}
if (what.weights)
weightsystems = data->weightsystems;
if (what.number)
number = data->number;
if (what.when)
when = data->when;
weightsystems = data.weights;
number = data.number;
when = data.when;
}
PasteState::~PasteState()
{
}
void PasteState::swap(dive_components what)
void PasteState::swap()
{
if (what.notes)
std::swap(notes, d->notes);
if (what.diveguide)
std::swap(diveguide, d->diveguide);
if (what.buddy)
std::swap(buddy, d->buddy);
if (what.suit)
std::swap(suit, d->suit);
if (what.rating)
std::swap(rating, d->rating);
if (what.visibility)
std::swap(visibility, d->visibility);
if (what.wavesize)
std::swap(wavesize, d->wavesize);
if (what.current)
std::swap(current, d->current);
if (what.surge)
std::swap(surge, d->surge);
if (what.chill)
std::swap(chill, d->chill);
if (what.divesite)
std::swap(divesite, d->dive_site);
if (what.tags)
std::swap(tags, d->tags);
if (what.cylinders)
std::swap(cylinders, d->cylinders);
if (what.weights)
std::swap(weightsystems, d->weightsystems);
if (what.number)
std::swap(number, d->number);
if (what.when)
std::swap(when, d->when);
if (notes.has_value())
std::swap(*notes, d.notes);
if (diveguide.has_value())
std::swap(*diveguide, d.diveguide);
if (buddy.has_value())
std::swap(*buddy, d.buddy);
if (suit.has_value())
std::swap(*suit, d.suit);
if (rating.has_value())
std::swap(*rating, d.rating);
if (visibility.has_value())
std::swap(*visibility, d.visibility);
if (wavesize.has_value())
std::swap(*wavesize, d.wavesize);
if (current.has_value())
std::swap(*current, d.current);
if (surge.has_value())
std::swap(*surge, d.surge);
if (chill.has_value())
std::swap(*chill, d.chill);
if (divesite.has_value() && *divesite != d.dive_site) {
auto old_ds = unregister_dive_from_dive_site(&d);
if (*divesite)
(*divesite)->add_dive(&d);
divesite = old_ds;
}
if (tags.has_value())
std::swap(*tags, d.tags);
if (cylinders.has_value())
std::swap(*cylinders, d.cylinders);
if (weightsystems.has_value())
std::swap(*weightsystems, d.weightsystems);
if (number.has_value())
std::swap(*number, d.number);
if (when.has_value())
std::swap(*when, d.when);
}
// ***** Paste *****
PasteDives::PasteDives(const dive *data, dive_components whatIn) : what(whatIn)
PasteDives::PasteDives(const dive_paste_data &data)
{
std::vector<dive *> selection = getDiveSelection();
dives.reserve(selection.size());
for (dive *d: selection)
dives.emplace_back(d, data, what);
dives.emplace_back(*d, data, dive_sites_changed);
setText(QStringLiteral("%1 [%2]").arg(Command::Base::tr("Paste onto %n dive(s)", "", dives.size())).arg(getListOfDives(selection)));
}
@ -739,32 +750,35 @@ void PasteDives::undo()
QVector<dive *> divesToNotify; // Remember dives so that we can send signals later
divesToNotify.reserve(dives.size());
for (PasteState &state: dives) {
divesToNotify.push_back(state.d);
state.swap(what);
state.d->invalidate_cache(); // Ensure that dive is written in git_save()
divesToNotify.push_back(&state.d);
state.swap();
state.d.invalidate_cache(); // Ensure that dive is written in git_save()
}
// Send signals.
// Send signals. We use the first data item to determine what changed
DiveField fields(DiveField::NONE);
fields.notes = what.notes;
fields.diveguide = what.diveguide;
fields.buddy = what.buddy;
fields.suit = what.suit;
fields.rating = what.rating;
fields.visibility = what.visibility;
fields.wavesize = what.wavesize;
fields.current = what.current;
fields.surge = what.surge;
fields.chill = what.chill;
fields.divesite = what.divesite;
fields.tags = what.tags;
fields.datetime = what.when;
fields.nr = what.number;
const PasteState &state = dives[0];
fields.notes = state.notes.has_value();
fields.diveguide = state.diveguide.has_value();
fields.buddy = state.buddy.has_value();
fields.suit = state.suit.has_value();
fields.rating = state.rating.has_value();
fields.visibility = state.visibility.has_value();
fields.wavesize = state.wavesize.has_value();
fields.current = state.current.has_value();
fields.surge = state.surge.has_value();
fields.chill = state.chill.has_value();
fields.divesite = !dive_sites_changed.empty();
fields.tags = state.tags.has_value();
fields.datetime = state.when.has_value();
fields.nr = state.number.has_value();
emit diveListNotifier.divesChanged(divesToNotify, fields);
if (what.cylinders)
if (state.cylinders.has_value())
emit diveListNotifier.cylindersReset(divesToNotify);
if (what.weights)
if (state.weightsystems.has_value())
emit diveListNotifier.weightsystemsReset(divesToNotify);
for (dive_site *ds: dive_sites_changed)
emit diveListNotifier.diveSiteDivesChanged(ds);
}
// Redo and undo do the same

43
commands/command_edit.h
View file

@ -287,34 +287,35 @@ public:
// Fields we have to remember to undo paste
struct PasteState {
dive *d;
dive_site *divesite;
std::string notes;
std::string diveguide;
std::string buddy;
std::string suit;
int rating;
int wavesize;
int visibility;
int current;
int surge;
int chill;
tag_list tags;
cylinder_table cylinders;
weightsystem_table weightsystems;
int number;
timestamp_t when;
dive &d;
std::optional<dive_site *> divesite;
std::optional<std::string> notes;
std::optional<std::string> diveguide;
std::optional<std::string> buddy;
std::optional<std::string> suit;
std::optional<int> rating;
std::optional<int> wavesize;
std::optional<int> visibility;
std::optional<int> current;
std::optional<int> surge;
std::optional<int> chill;
std::optional<tag_list> tags;
std::optional<cylinder_table> cylinders;
std::optional<weightsystem_table> weightsystems;
std::optional<int> number;
std::optional<timestamp_t> when;
PasteState(dive *d, const dive *data, dive_components what); // Read data from dive data for dive d
PasteState(dive &d, const dive_paste_data &data, std::vector<dive_site *> &changed_dive_sites);
~PasteState();
void swap(dive_components what); // Exchange values here and in dive
void swap(); // Exchange values here and in dive
};
class PasteDives : public Base {
dive_components what;
dive_paste_data data;
std::vector<PasteState> dives;
std::vector<dive_site *> dive_sites_changed;
public:
PasteDives(const dive *d, dive_components what);
PasteDives(const dive_paste_data &data);
private:
void undo() override;
void redo() override;