Archives
/
notcurses
mirror of https://github.com/dankamongmen/notcurses
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[C++] Deal with widgets grabbing full ownership of Panel

Browse Source 
Fixes: https://github.com/dankamongmen/notcurses/issues/1009

Whenever a widget is created with its `*_create` function it currently
claims full ownership of the passed panel, including its destruction.
However, the C++ wrapper around the panel is not aware of this and will
attempt to destroy the native panel in the destructor, leading to
segfaults.

Fix this by introduction of a `Widget` class which contains the logic to
properly modify the `Panel` instance to not double-destroy the native
panel.  The solution is a bit fragile since the `Panel` instance is left
intact (we can't free it for the user) in a state that's safe for the
C++ wrapper, but calling any C function via the wrapper **will** pass a
`NULL` pointer in the panel argument - therefore the C functions MUST be
proofed against this.  The proofing belongs in the C backend code since
this protects also C and other language binding users from such abuse.

The Widget class will first verify that the passed `Plane` instance
hasn't already been "disowned" and will throw an exception to the effect
if it was.  Next, it will proceed to take over ownership of the native
panel instance and mark the passed `Panel` as "invalid" (i.e. not owning
any native panel instance anymore)

The above changes require modification of `Panel` instances and so all
the widget constructors taking `const*` or `const&` have been removed
from widget classes.
pull/1040/head
Marek Habersack 4 years ago committed by Nick Black
parent 7241e0a5e1
commit a29bfe9c42
12 changed files with 173 additions and 140 deletions
Show Stats Download Patch File Download Diff File

45
include/ncpp/FDPlane.hh
Unescape Escape View File

@ -5,51 +5,38 @@
#include "Utilities.hh"
#include "Plane.hh"
#include "Widget.hh"
namespace ncpp
{
class NCPP_API_EXPORT FDPlane : public Root
class NCPP_API_EXPORT FDPlane : public Widget
{
public:
static ncfdplane_options default_options;
public:
explicit FDPlane (Plane* n, int fd, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: FDPlane (n, fd, nullptr, cbfxn, donecbfxn)
explicit FDPlane (Plane *plane, int fd, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: FDPlane (plane, fd, nullptr, cbfxn, donecbfxn)
{}
explicit FDPlane (const Plane* n, int fd, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: FDPlane (n, fd, nullptr, cbfxn, donecbfxn)
{}
explicit FDPlane (Plane* n, int fd, ncfdplane_options *opts = nullptr, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: FDPlane (static_cast<const Plane*>(n), fd, opts, cbfxn, donecbfxn)
{}
explicit FDPlane (const Plane* n, int fd, ncfdplane_options *opts = nullptr, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Root (Utilities::get_notcurses_cpp (n))
explicit FDPlane (Plane *plane, int fd, ncfdplane_options *opts = nullptr, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Widget (Utilities::get_notcurses_cpp (plane))
{
if (n == nullptr)
throw invalid_argument ("'n' must be a valid pointer");
create_fdplane (const_cast<Plane&>(*n), fd, opts, cbfxn, donecbfxn);
ensure_valid_plane (plane);
create_fdplane (*plane, fd, opts, cbfxn, donecbfxn);
take_plane_ownership (plane);
}
explicit FDPlane (const Plane& n, int fd, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: FDPlane (n, fd, nullptr, cbfxn, donecbfxn)
{}
explicit FDPlane (Plane& n, int fd, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: FDPlane (n, fd, nullptr, cbfxn, donecbfxn)
{}
explicit FDPlane (Plane& n, int fd, ncfdplane_options *opts = nullptr, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: FDPlane (static_cast<Plane const&>(n), fd, opts, cbfxn, donecbfxn)
explicit FDPlane (Plane &plane, int fd, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: FDPlane (plane, fd, nullptr, cbfxn, donecbfxn)
{}
explicit FDPlane (const Plane& n, int fd, ncfdplane_options *opts = nullptr, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Root (Utilities::get_notcurses_cpp (n))
explicit FDPlane (Plane &plane, int fd, ncfdplane_options *opts = nullptr, ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Widget (Utilities::get_notcurses_cpp (plane))
{
create_fdplane (const_cast<Plane&>(n), fd, opts, cbfxn, donecbfxn);
ensure_valid_plane (plane);
create_fdplane (plane, fd, opts, cbfxn, donecbfxn);
take_plane_ownership (plane);
}
~FDPlane ()

19
include/ncpp/MultiSelector.hh
Unescape Escape View File

@ -6,33 +6,30 @@
#include "NCAlign.hh"
#include "Plane.hh"
#include "Utilities.hh"
#include "Widget.hh"
namespace ncpp
{
class NCPP_API_EXPORT MultiSelector : public Root
class NCPP_API_EXPORT MultiSelector : public Widget
{
public:
static ncmultiselector_options default_options;
public:
explicit MultiSelector (Plane *plane, const ncmultiselector_options *opts = nullptr)
: MultiSelector (static_cast<const Plane*>(plane), opts)
{}
explicit MultiSelector (Plane const* plane, const ncmultiselector_options *opts = nullptr)
: Root (Utilities::get_notcurses_cpp (plane))
: Widget (Utilities::get_notcurses_cpp (plane))
{
ensure_valid_plane (plane);
common_init (Utilities::to_ncplane (plane), opts);
take_plane_ownership (plane);
}
explicit MultiSelector (Plane &plane, const ncmultiselector_options *opts = nullptr)
: MultiSelector (static_cast<Plane const&>(plane), opts)
{}
explicit MultiSelector (Plane const& plane, const ncmultiselector_options *opts = nullptr)
: Root (Utilities::get_notcurses_cpp (plane))
: Widget (Utilities::get_notcurses_cpp (plane))
{
ensure_valid_plane (plane);
common_init (Utilities::to_ncplane (plane), opts);
take_plane_ownership (plane);
}
~MultiSelector ()

46
include/ncpp/Plane.hh
Unescape Escape View File

@ -10,16 +10,31 @@
#include "Root.hh"
#include "Cell.hh"
#include "Reel.hh"
#include "CellStyle.hh"
#include "NCAlign.hh"
#include "NCBox.hh"
namespace ncpp
{
class NcReel;
class NCPP_API_EXPORT Plane : public Root
{
public:
Plane (Plane&& other)
: Root (nullptr)
{
unmap_plane (&other);
plane = other.plane;
is_stdplane = other.is_stdplane;
map_plane (plane, this);
other.plane = nullptr;
other.is_stdplane = false;
}
Plane (Plane const& other)
: Root (nullptr)
{
@ -61,7 +76,7 @@ namespace ncpp
yoff,
xoff,
opaque,
nullptr
nullptr
);
if (plane == nullptr)
@ -107,7 +122,7 @@ namespace ncpp
~Plane () noexcept
{
if (is_stdplane)
if (is_stdplane || plane == nullptr)
return;
if (!is_notcurses_stopped ())
@ -956,10 +971,7 @@ namespace ncpp
return static_cast<T*>(get_userptr ());
}
NcReel* ncreel_create (const ncreel_options *popts = nullptr) const
{
return new NcReel (this, popts);
}
NcReel* ncreel_create (const ncreel_options *popts = nullptr);
// Some Cell APIs go here since they act on individual panels even though it may seem weird at points (e.g.
// release)
@ -1124,6 +1136,11 @@ namespace ncpp
return ncplane_bg_default_p (plane);
}
bool is_valid () const noexcept
{
return plane != nullptr;
}
protected:
explicit Plane (ncplane *_plane, bool _is_stdplane)
: Root (nullptr),
@ -1134,6 +1151,15 @@ namespace ncpp
throw invalid_argument ("_plane must be a valid pointer");
}
void release_native_plane () noexcept
{
if (plane == nullptr)
return;
unmap_plane (this);
plane = nullptr;
}
static void unmap_plane (Plane *p) noexcept;
private:
@ -1146,7 +1172,7 @@ namespace ncpp
yoff,
xoff,
opaque,
nullptr
nullptr
);
if (ret == nullptr)
@ -1166,7 +1192,7 @@ namespace ncpp
cols,
opaque,
nullptr,
nullptr,
nullptr,
0
};
ncplane *ret = ncplane_create (
@ -1200,6 +1226,8 @@ namespace ncpp
friend class NotCurses;
friend class NcReel;
friend class Tablet;
friend class Widget;
template<typename TPlot, typename TCoord> friend class PlotBase;
};
}
#endif

13
include/ncpp/Plot.hh
Unescape Escape View File

@ -8,11 +8,12 @@
#include "NCAlign.hh"
#include "Plane.hh"
#include "Utilities.hh"
#include "Widget.hh"
namespace ncpp
{
template<typename TPlot, typename TCoord>
class NCPP_API_EXPORT PlotBase : public Root
class NCPP_API_EXPORT PlotBase : public Widget
{
static constexpr bool is_double = std::is_same_v<TCoord,double>;
static constexpr bool is_uint64 = std::is_same_v<TCoord,uint64_t>;
@ -58,7 +59,7 @@ namespace ncpp
protected:
explicit PlotBase (Plane *plane, const ncplot_options *opts, TCoord miny = 0, TCoord maxy = 0)
: Root (Utilities::get_notcurses_cpp (plane))
: Widget (Utilities::get_notcurses_cpp (plane))
{
static_assert (is_double || is_uint64, "PlotBase must be parameterized with either 'double' or 'uint64_t' types");
if constexpr (is_double) {
@ -67,8 +68,10 @@ namespace ncpp
static_assert (std::is_same_v<TPlot, ncuplot>, "ncuplot must be used for a plot using uint64_t coordinates");
}
if (plane == nullptr)
throw invalid_argument ("'plane' must be a valid pointer");
ensure_valid_plane (plane);
if (!plane->is_valid ())
throw invalid_argument ("Invalid Plane object passed in 'plane'. Widgets must not reuse the same plane.");
if (opts == nullptr)
throw invalid_argument ("'opts' must be a valid pointer");
@ -81,6 +84,8 @@ namespace ncpp
if (plot == nullptr)
throw init_error ("Notcurses failed to create a new plot");
take_plane_ownership (plane);
}
~PlotBase ()

30
include/ncpp/Reader.hh
Unescape Escape View File

@ -6,33 +6,27 @@
#include "NCAlign.hh"
#include "Plane.hh"
#include "Utilities.hh"
#include "Widget.hh"
namespace ncpp
{
class NCPP_API_EXPORT Reader : public Root
class NCPP_API_EXPORT Reader : public Widget
{
public:
explicit Reader (Plane *p, const ncreader_options *opts)
: Reader (static_cast<const Plane*>(p), opts)
{}
explicit Reader (Plane const* p, const ncreader_options *opts)
: Root (Utilities::get_notcurses_cpp (p))
explicit Reader (Plane *plane, const ncreader_options *opts)
: Widget (Utilities::get_notcurses_cpp (plane))
{
if (p == nullptr)
throw invalid_argument ("'plane' must be a valid pointer");
common_init (Utilities::to_ncplane (p), opts);
ensure_valid_plane (plane);
common_init (Utilities::to_ncplane (plane), opts);
take_plane_ownership (plane);
}
explicit Reader (Plane &p, const ncreader_options *opts)
: Reader (static_cast<Plane const&>(p), opts)
{}
explicit Reader (Plane const& p, const ncreader_options *opts)
: Root (Utilities::get_notcurses_cpp (p))
explicit Reader (Plane &plane, const ncreader_options *opts)
: Widget (Utilities::get_notcurses_cpp (plane))
{
common_init (Utilities::to_ncplane (p), opts);
ensure_valid_plane (plane);
common_init (Utilities::to_ncplane (plane), opts);
take_plane_ownership (plane);
}
~Reader ()

19
include/ncpp/Reel.hh
Unescape Escape View File

@ -7,35 +7,32 @@
#include "Tablet.hh"
#include "Plane.hh"
#include "Utilities.hh"
#include "Widget.hh"
namespace ncpp
{
class NCPP_API_EXPORT NcReel : public Root
class NCPP_API_EXPORT NcReel : public Widget
{
public:
static ncreel_options default_options;
explicit NcReel (Plane &plane, const ncreel_options *popts = nullptr)
: NcReel (static_cast<Plane const&>(plane), popts)
{}
explicit NcReel (Plane const&plane, const ncreel_options *popts = nullptr)
: Root (Utilities::get_notcurses_cpp (plane))
: Widget (Utilities::get_notcurses_cpp (plane))
{
ensure_valid_plane (plane);
common_init (Utilities::to_ncplane (plane), popts);
take_plane_ownership (plane);
}
explicit NcReel (Plane *plane, const ncreel_options *popts = nullptr)
: NcReel (static_cast<const Plane*>(plane), popts)
{}
explicit NcReel (const Plane *plane, const ncreel_options *popts = nullptr)
: Root (Utilities::get_notcurses_cpp (plane))
: Widget (Utilities::get_notcurses_cpp (plane))
{
if (plane == nullptr)
throw invalid_argument ("'plane' must be a valid pointer");
ensure_valid_plane (plane);
common_init (Utilities::to_ncplane (plane), popts);
take_plane_ownership (plane);
}
~NcReel ()

20
include/ncpp/Selector.hh
Unescape Escape View File

@ -6,35 +6,33 @@
#include "NCAlign.hh"
#include "Plane.hh"
#include "Utilities.hh"
#include "Widget.hh"
namespace ncpp
{
class NCPP_API_EXPORT Selector : public Root
class NCPP_API_EXPORT Selector : public Widget
{
public:
static ncselector_options default_options;
public:
explicit Selector (Plane *plane, const ncselector_options *opts = nullptr)
: Selector (static_cast<const Plane*>(plane), opts)
{}
explicit Selector (Plane const* plane, const ncselector_options *opts = nullptr)
: Root (Utilities::get_notcurses_cpp (plane))
: Widget (Utilities::get_notcurses_cpp (plane))
{
if (plane == nullptr)
throw invalid_argument ("'plane' must be a valid pointer");
ensure_valid_plane (plane);
common_init (Utilities::to_ncplane (plane), opts);
take_plane_ownership (plane);
}
explicit Selector (Plane &plane, const ncselector_options *opts = nullptr)
: Selector (static_cast<Plane const&>(plane), opts)
{}
explicit Selector (Plane const& plane, const ncselector_options *opts = nullptr)
: Root (Utilities::get_notcurses_cpp (plane))
: Widget (Utilities::get_notcurses_cpp (plane))
{
ensure_valid_plane (plane);
common_init (Utilities::to_ncplane (plane), opts);
take_plane_ownership (plane);
}
~Selector ()

54
include/ncpp/Subproc.hh
Unescape Escape View File

@ -5,67 +5,47 @@
#include "Root.hh"
#include "Plane.hh"
#include "Widget.hh"
#include "Utilities.hh"
namespace ncpp
{
class NCPP_API_EXPORT Subproc : public Root
class NCPP_API_EXPORT Subproc : public Widget
{
public:
static ncsubproc_options default_options;
public:
explicit Subproc (Plane* n, const char* bin, bool use_path = true,
explicit Subproc (Plane* plane, const char* bin, bool use_path = true,
char* const arg[] = nullptr, char* const env[] = nullptr,
ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Subproc (n, bin, nullptr, use_path, arg, env, cbfxn, donecbfxn)
: Subproc (plane, bin, nullptr, use_path, arg, env, cbfxn, donecbfxn)
{}
explicit Subproc (const Plane* n, const char* bin, bool use_path = true,
explicit Subproc (Plane* plane, const char* bin, const ncsubproc_options* opts, bool use_path = true,
char* const arg[] = nullptr, char* const env[] = nullptr,
ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Subproc (n, bin, nullptr, use_path, arg, env, cbfxn, donecbfxn)
{}
explicit Subproc (Plane* n, const char* bin, const ncsubproc_options* opts, bool use_path = true,
char* const arg[] = nullptr, char* const env[] = nullptr,
ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Subproc (static_cast<const Plane*>(n), bin, opts, use_path, arg, env, cbfxn, donecbfxn)
{}
explicit Subproc (const Plane* n, const char* bin, const ncsubproc_options* opts, bool use_path = true,
char* const arg[] = nullptr, char* const env[] = nullptr,
ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Root (Utilities::get_notcurses_cpp (n))
: Widget (Utilities::get_notcurses_cpp (plane))
{
if (n == nullptr)
throw invalid_argument ("'n' must be a valid pointer");
create_subproc (const_cast<Plane&>(*n), bin, opts, use_path, arg, env, cbfxn, donecbfxn);
ensure_valid_plane (plane);
create_subproc (*plane, bin, opts, use_path, arg, env, cbfxn, donecbfxn);
take_plane_ownership (plane);
}
explicit Subproc (Plane const& n, const char* bin, bool use_path = true,
char* const arg[] = nullptr, char* const env[] = nullptr,
ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Subproc (n, bin, nullptr, use_path, arg, env, cbfxn, donecbfxn)
{}
explicit Subproc (Plane& n, const char* bin, bool use_path = true,
char* const arg[] = nullptr, char* const env[] = nullptr,
ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Subproc (n, bin, nullptr, use_path, arg, env, cbfxn, donecbfxn)
{}
explicit Subproc (Plane& n, const char* bin, const ncsubproc_options* opts, bool use_path = true,
explicit Subproc (Plane& plane, const char* bin, bool use_path = true,
char* const arg[] = nullptr, char* const env[] = nullptr,
ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Subproc (static_cast<Plane const&>(n), bin, opts, use_path, arg, env, cbfxn, donecbfxn)
: Subproc (plane, bin, nullptr, use_path, arg, env, cbfxn, donecbfxn)
{}
explicit Subproc (const Plane& n, const char* bin, const ncsubproc_options* opts, bool use_path = true,
explicit Subproc (Plane& plane, const char* bin, const ncsubproc_options* opts, bool use_path = true,
char* const arg[] = nullptr, char* const env[] = nullptr,
ncfdplane_callback cbfxn = nullptr, ncfdplane_done_cb donecbfxn = nullptr)
: Root (Utilities::get_notcurses_cpp (n))
: Widget (Utilities::get_notcurses_cpp (plane))
{
create_subproc (const_cast<Plane&>(n), bin, opts, use_path, arg, env, cbfxn, donecbfxn);
ensure_valid_plane (plane);
create_subproc (plane, bin, opts, use_path, arg, env, cbfxn, donecbfxn);
take_plane_ownership (plane);
}
~Subproc ()

43
include/ncpp/Widget.hh
Unescape Escape View File

@ -0,0 +1,43 @@
#ifndef __NCPP_WIDGET_HH
#define __NCPP_WIDGET_HH
#include "Root.hh"
#include "Plane.hh"
namespace ncpp
{
class NCPP_API_EXPORT Widget : public Root
{
protected:
explicit Widget (NotCurses *ncinst)
: Root (ncinst)
{}
void ensure_valid_plane (Plane *plane) const
{
if (plane == nullptr)
throw invalid_argument ("'plane' must be a valid pointer");
ensure_valid_plane (*plane);
}
void ensure_valid_plane (Plane &plane) const
{
if (!plane.is_valid ())
throw invalid_argument ("Invalid Plane object passed in 'plane'. Widgets must not reuse the same plane.");
}
void take_plane_ownership (Plane *plane) const
{
if (plane == nullptr)
return;
take_plane_ownership (*plane);
}
void take_plane_ownership (Plane &plane) const
{
plane.release_native_plane ();
}
};
}
#endif // __NCPP_WIDGET_HH

6
src/libcpp/Plane.cc
Unescape Escape View File

@ -1,4 +1,5 @@
#include <ncpp/Plane.hh>
#include <ncpp/Reel.hh>
#include <ncpp/internal/Helpers.hh>
using namespace ncpp;
@ -28,3 +29,8 @@ void Plane::unmap_plane (Plane *p) noexcept
internal::Helpers::remove_map_entry (plane_map, plane_map_mutex, p->plane);
}
NcReel* Plane::ncreel_create (const ncreel_options *popts)
{
return new NcReel (this, popts);
}

2
src/poc/ncpp_build.cpp
Unescape Escape View File

@ -27,14 +27,12 @@ int run ()
const char *ncver = nc.version ();
{
/*
Plane p1 (1, 1, 0, 0);
Plot plot1 (p1);
Plane p2 (1, 1, 0, 0);
PlotU plot2 (p2);
Plane p3 (1, 1, 0, 0);
PlotD plot3 (p3);
*/
}
nc.stop ();

16
src/poc/ncpp_build_exceptions.cpp
Unescape Escape View File

@ -27,14 +27,14 @@ int run ()
NotCurses nc;
const char *ncver = nc.version ();
{
/*
Plane plane (1, 1, 0, 0);
Plot plot1 (plane);
PlotU plot2 (plane);
PlotD plot3 (plane);
*/
}
{
Plane p1 (1, 1, 0, 0);
Plot plot1 (p1);
Plane p2 (1, 1, 0, 0);
PlotU plot2 (p2);
Plane p3 (1, 1, 0, 0);
PlotD plot3 (p3);
}
nc.stop ();

Write Preview
Loading…
Cancel
Save