From c5d6e0f44f6429935eb06d0d76c048d0355f5602 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 6 Nov 2022 12:18:27 +0100 Subject: [PATCH] core: make owning pointers a top-level features The undo-code uses owning pointers based on std::unique_ptr to manage lifetime of C-objects. Since these are generally useful, move them from the undo-code to the core-code. In fact, this eliminates one instance of code duplication. Signed-off-by: Berthold Stoeger --- Subsurface-mobile.pro | 1 + commands/command.h | 4 ++-- commands/command_base.h | 21 +---------------- core/CMakeLists.txt | 1 + core/owning_ptrs.h | 41 +++++++++++++++++++++++++++++++++ desktop-widgets/profilewidget.h | 9 ++------ mobile-widgets/qmlmanager.cpp | 4 ++-- 7 files changed, 50 insertions(+), 31 deletions(-) create mode 100644 core/owning_ptrs.h diff --git a/Subsurface-mobile.pro b/Subsurface-mobile.pro index 3e695ce98..31c104a01 100644 --- a/Subsurface-mobile.pro +++ b/Subsurface-mobile.pro @@ -206,6 +206,7 @@ HEADERS += \ core/extradata.h \ core/git-access.h \ core/globals.h \ + core/owning_ptrs.h \ core/pref.h \ core/profile.h \ core/qthelper.h \ diff --git a/commands/command.h b/commands/command.h index e7e4449c0..7010162f5 100644 --- a/commands/command.h +++ b/commands/command.h @@ -30,8 +30,8 @@ void clear(); // Reset the undo stack. Delete all commands. void setClean(); // Call after save - this marks a state where no changes need to be saved. bool isClean(); // Any changes need to be saved? QAction *undoAction(QObject *parent); // Create an undo action. -QAction *redoAction(QObject *parent); // Create an redo action. -QString changesMade(); // return a string with the texts from all commands on the undo stack -> for commit message +QAction *redoAction(QObject *parent); // Create a redo action. +QString changesMade(); // Return a string with the texts from all commands on the undo stack -> for commit message. bool placingCommand(); // Currently executing a new command -> might not have to update the field the user just edited. // 2) Dive-list related commands diff --git a/commands/command_base.h b/commands/command_base.h index 8c905d09a..dacaee89f 100644 --- a/commands/command_base.h +++ b/commands/command_base.h @@ -7,6 +7,7 @@ #include "core/divesite.h" #include "core/trip.h" #include "core/dive.h" +#include "core/owning_ptrs.h" #include #include // For Q_DECLARE_TR_FUNCTIONS @@ -153,26 +154,6 @@ QVector stdToQt(const std::vector &v) // We put everything in a namespace, so that we can shorten names without polluting the global namespace namespace Command { -// Classes used to automatically call the appropriate free_*() function for owning pointers that go out of scope. -struct DiveDeleter { - void operator()(dive *d) { free_dive(d); } -}; -struct TripDeleter { - void operator()(dive_trip *t) { free_trip(t); } -}; -struct DiveSiteDeleter { - void operator()(dive_site *ds) { free_dive_site(ds); } -}; -struct EventDeleter { - void operator()(event *ev) { free(ev); } -}; - -// Owning pointers to dive, dive_trip, dive_site and event objects. -typedef std::unique_ptr OwningDivePtr; -typedef std::unique_ptr OwningTripPtr; -typedef std::unique_ptr OwningDiveSitePtr; -typedef std::unique_ptr OwningEventPtr; - // This is the base class of all commands. // It defines the Qt-translation functions class Base : public QUndoCommand { diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt index ba5b2ee23..f069891a1 100644 --- a/core/CMakeLists.txt +++ b/core/CMakeLists.txt @@ -132,6 +132,7 @@ set(SUBSURFACE_CORE_LIB_SRCS metrics.cpp metrics.h ostctools.c + owning_ptrs.h parse-gpx.cpp parse-xml.c parse.c diff --git a/core/owning_ptrs.h b/core/owning_ptrs.h new file mode 100644 index 000000000..6f591d144 --- /dev/null +++ b/core/owning_ptrs.h @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-2.0 +// Convenience classes defining owning pointers to C-objects that +// automatically clean up the objects if the pointers go out of +// scope. Based on unique_ptr<>. +// In the future, we should replace these by real destructors. +#ifndef OWNING_PTR_H +#define OWNING_PTR_H + +#include +#include + +struct dive; +struct dive_trip; +struct dive_site; +struct event; + +extern "C" void free_dive(struct dive *); +extern "C" void free_trip(struct dive_trip *); +extern "C" void free_dive_site(struct dive_site *); + +// Classes used to automatically call the appropriate free_*() function for owning pointers that go out of scope. +struct DiveDeleter { + void operator()(dive *d) { free_dive(d); } +}; +struct TripDeleter { + void operator()(dive_trip *t) { free_trip(t); } +}; +struct DiveSiteDeleter { + void operator()(dive_site *ds) { free_dive_site(ds); } +}; +struct EventDeleter { + void operator()(event *ev) { free(ev); } +}; + +// Owning pointers to dive, dive_trip, dive_site and event objects. +using OwningDivePtr = std::unique_ptr; +using OwningTripPtr = std::unique_ptr; +using OwningDiveSitePtr = std::unique_ptr; +using OwningEventPtr = std::unique_ptr; + +#endif diff --git a/desktop-widgets/profilewidget.h b/desktop-widgets/profilewidget.h index a217168bb..abdf01795 100644 --- a/desktop-widgets/profilewidget.h +++ b/desktop-widgets/profilewidget.h @@ -5,10 +5,10 @@ #define PROFILEWIDGET_H #include "ui_profilewidget.h" +#include "core/owning_ptrs.h" #include "core/subsurface-qt/divelistnotifier.h" #include -#include struct dive; class ProfileWidget2; @@ -40,11 +40,6 @@ slots: void stopRemoved(int count); void stopMoved(int count); private: - // The same code is in command/command_base.h. Should we make that a global feature? - struct DiveDeleter { - void operator()(dive *d) { free_dive(d); } - }; - std::unique_ptr emptyView; std::vector toolbarActions; Ui::ProfileWidget ui; @@ -53,7 +48,7 @@ private: void editDive(); void exitEditMode(); void rotateDC(int dir); - std::unique_ptr editedDive; + OwningDivePtr editedDive; int editedDc; dive *originalDive; bool placingCommand; diff --git a/mobile-widgets/qmlmanager.cpp b/mobile-widgets/qmlmanager.cpp index 4c46a8434..ed4016827 100644 --- a/mobile-widgets/qmlmanager.cpp +++ b/mobile-widgets/qmlmanager.cpp @@ -897,7 +897,7 @@ void QMLManager::refreshDiveList() // The following structure describes such a change caused by a dive edit. // Hopefully, we can remove this in due course by using finer-grained undo-commands. struct DiveSiteChange { - Command::OwningDiveSitePtr createdDs; // not-null if we created a dive site. + OwningDiveSitePtr createdDs; // not-null if we created a dive site. dive_site *editDs = nullptr; // not-null if we are supposed to edit an existing dive site. location_t location = zero_location; // new value of the location if we edit an existing dive site. @@ -1181,7 +1181,7 @@ void QMLManager::commitChanges(QString diveId, QString number, QString date, QSt QStringLiteral("state :'%1'\n").arg(state); } - Command::OwningDivePtr d_ptr(alloc_dive()); // Automatically delete dive if we exit early! + OwningDivePtr d_ptr(alloc_dive()); // Automatically delete dive if we exit early! dive *d = d_ptr.get(); copy_dive(orig, d);