Developer forums (C::B DEVELOPMENT STRICTLY!) > Development
wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
ollydbg:
--- Code: ---
src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp | 11 +++++++----
src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp | 4 +++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
index bbd3d967..3b9bfcbb 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
@@ -640,14 +640,15 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
wxMessageBox(_("wxAuiManager can't be added to a sizer. Add panels first."));
return false;
}
-
- if ( !wxDynamicCast(Parent->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxWindow) )
+ wxFrame * tempFrame = new wxFrame(0,-1,wxEmptyString);
+ if ( !wxDynamicCast(Parent->BuildPreview(tempFrame,0),wxWindow) )
{
if ( ShowMessage )
wxMessageBox(_("wxAuiManager can only be added to a wxWindow descendant."));
+ tempFrame->Destroy();
return false;
}
-
+ tempFrame->Destroy();
return wxsParent::OnCanAddToParent(Parent,ShowMessage);
}
@@ -665,7 +666,8 @@ void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
if ( ChildExtra->m_FirstAdd )
{
ChildExtra->m_FirstAdd = false;
- if ( wxDynamicCast(Child->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxAuiToolBar) )
+ wxFrame * tempFrame = new wxFrame(0,-1,wxEmptyString);
+ if ( wxDynamicCast(Child->BuildPreview(tempFrame,0),wxAuiToolBar) )
{
ChildExtra->m_StandardPane = wxsAuiPaneInfoExtra::ToolbarPane;
ChildExtra->m_DockableFlags = wxsAuiDockableProperty::Dockable;
@@ -680,6 +682,7 @@ void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
if ( ChildExtra->m_Layer == 0 ) ChildExtra->m_Layer = 10;
NotifyPropertyChange();
}
+ tempFrame->Destroy();
}
}
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
index 66b67edc..7f0f8eff 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
@@ -126,7 +126,9 @@ wxsAuiToolBar::~wxsAuiToolBar()
bool wxsAuiToolBar::OnCanAddChild(wxsItem* Item,bool ShowMessage)
{
- bool IsControl = wxDynamicCast(Item->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxControl);
+ wxFrame * tempFrame = new wxFrame(0,-1,wxEmptyString);
+ bool IsControl = wxDynamicCast(Item->BuildPreview(tempFrame,0),wxControl);
+ tempFrame->Destroy();
bool IsAuiToolBarItem = Item->GetClassName().Contains(_T("wxAuiToolBar"));
if ( !IsControl && !IsAuiToolBarItem )
{
--- End code ---
This is the full patch. OK to commit to trunk?
sodev:
Dont use 0 but nullptr, dont use -1 but wxID_ANY, dont use raw pointer but something managed, something like:
--- Code: ---auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString), [](wxFrame* frame) { frame->Destroy(); });
--- End code ---
This way you dont need explicit Destroy() calls. Would be better to use std::unique_ptr, but there the deleter is part of the signature and i currently dont know how to use that with a nice lambda expression and dont have a compiler on my cell phone to play around with :D.
But beside that, why does the code even create these temporary frames and is it really safe to destroy them right after the method call? Why to create them at all then?
ollydbg:
--- Quote from: sodev on February 13, 2019, 05:12:32 am ---Dont use 0 but nullptr, dont use -1 but wxID_ANY, dont use raw pointer but something managed, something like:
--- Code: ---auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString), [](wxFrame* frame) { frame->Destroy(); });
--- End code ---
This way you dont need explicit Destroy() calls. Would be better to use std::unique_ptr, but there the deleter is part of the signature and i currently dont know how to use that with a nice lambda expression and dont have a compiler on my cell phone to play around with :D.
--- End quote ---
Thanks for the suggestion, and in-fact I'm not good at writing lambda expressions :), here is improved patch as your suggested.
--- Code: --- src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp | 9 ++++++---
src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp | 4 +++-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
index bbd3d967..3cf86b3e 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
@@ -640,8 +640,9 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
wxMessageBox(_("wxAuiManager can't be added to a sizer. Add panels first."));
return false;
}
-
- if ( !wxDynamicCast(Parent->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxWindow) )
+ auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
+ [](wxFrame* frame) {frame->Destroy();}); //deleter
+ if ( !wxDynamicCast(Parent->BuildPreview(tempFrame.get(),0),wxWindow) )
{
if ( ShowMessage )
wxMessageBox(_("wxAuiManager can only be added to a wxWindow descendant."));
@@ -665,7 +666,9 @@ void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
if ( ChildExtra->m_FirstAdd )
{
ChildExtra->m_FirstAdd = false;
- if ( wxDynamicCast(Child->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxAuiToolBar) )
+ auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
+ [](wxFrame* frame) {frame->Destroy();}); //deleter
+ if ( wxDynamicCast(Child->BuildPreview(tempFrame.get(),0),wxAuiToolBar) )
{
ChildExtra->m_StandardPane = wxsAuiPaneInfoExtra::ToolbarPane;
ChildExtra->m_DockableFlags = wxsAuiDockableProperty::Dockable;
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
index 66b67edc..af2adb70 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
@@ -126,7 +126,9 @@ wxsAuiToolBar::~wxsAuiToolBar()
bool wxsAuiToolBar::OnCanAddChild(wxsItem* Item,bool ShowMessage)
{
- bool IsControl = wxDynamicCast(Item->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxControl);
+ auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
+ [](wxFrame* frame) {frame->Destroy();}); //deleter
+ bool IsControl = wxDynamicCast(Item->BuildPreview(tempFrame.get(),0),wxControl);
bool IsAuiToolBarItem = Item->GetClassName().Contains(_T("wxAuiToolBar"));
if ( !IsControl && !IsAuiToolBarItem )
{
--- End code ---
--- Quote ---But beside that, why does the code even create these temporary frames and is it really safe to destroy them right after the method call? Why to create them at all then?
--- End quote ---
I don't know such logic here either. :(
oBFusCATed:
Can you add comments what is the purpose of the shared pointers? Why aren't you using a unique_ptr? I don't see any sharing happening here.
Can you please not use auto? In this case it doesn't make sense to use it.
ollydbg:
--- Quote from: oBFusCATed on February 13, 2019, 09:22:56 am ---Can you add comments what is the purpose of the shared pointers? Why aren't you using a unique_ptr? I don't see any sharing happening here.
--- End quote ---
Oh, thanks for the review, it is a copy-paste error, it definitely should be std::unique_ptr.
--- Quote ---Can you please not use auto? In this case it doesn't make sense to use it.
--- End quote ---
Here is the updated patch, any comments?
--- Code: --- .../contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp | 19 ++++++++++++++++---
.../contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp | 13 ++++++++++++-
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
index bbd3d967..5a66636c 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
@@ -27,6 +27,16 @@
using namespace wxsFlags;
+struct TempFrameDeleter
+{
+ void operator()(wxFrame* frame)
+ {
+ frame->Destroy();
+ }
+};
+
+using TempFrameUniquePtr = std::unique_ptr<wxFrame, TempFrameDeleter>;
+
namespace
{
#include "../images/wxsAuiManager16.xpm"
@@ -640,8 +650,8 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
wxMessageBox(_("wxAuiManager can't be added to a sizer. Add panels first."));
return false;
}
-
- if ( !wxDynamicCast(Parent->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxWindow) )
+ TempFrameUniquePtr tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString));
+ if ( !wxDynamicCast(Parent->BuildPreview(tempFrame.get(),0),wxWindow) )
{
if ( ShowMessage )
wxMessageBox(_("wxAuiManager can only be added to a wxWindow descendant."));
@@ -651,6 +661,8 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
return wxsParent::OnCanAddToParent(Parent,ShowMessage);
}
+
+
void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
{
wxsParent::OnAddChildQPP(Child,QPP);
@@ -665,7 +677,8 @@ void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
if ( ChildExtra->m_FirstAdd )
{
ChildExtra->m_FirstAdd = false;
- if ( wxDynamicCast(Child->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxAuiToolBar) )
+ TempFrameUniquePtr tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString));
+ if ( wxDynamicCast(Child->BuildPreview(tempFrame.get(),0),wxAuiToolBar) )
{
ChildExtra->m_StandardPane = wxsAuiPaneInfoExtra::ToolbarPane;
ChildExtra->m_DockableFlags = wxsAuiDockableProperty::Dockable;
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
index 66b67edc..3870d1cb 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
@@ -32,6 +32,16 @@
using namespace wxsFlags;
+struct TempFrameDeleter
+{
+ void operator()(wxFrame* frame)
+ {
+ frame->Destroy();
+ }
+};
+
+using TempFrameUniquePtr = std::unique_ptr<wxFrame, TempFrameDeleter>;
+
//(*Headers(wxsAuiToolBarParentQP)
#include <wx/checkbox.h>
#include <wx/sizer.h>
@@ -126,7 +136,8 @@ wxsAuiToolBar::~wxsAuiToolBar()
bool wxsAuiToolBar::OnCanAddChild(wxsItem* Item,bool ShowMessage)
{
- bool IsControl = wxDynamicCast(Item->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxControl);
+ TempFrameUniquePtr tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString));
+ bool IsControl = wxDynamicCast(Item->BuildPreview(tempFrame.get(),0),wxControl);
bool IsAuiToolBarItem = Item->GetClassName().Contains(_T("wxAuiToolBar"));
if ( !IsControl && !IsAuiToolBarItem )
{
--- End code ---
EDIT:
I don't think add the same part in two cpp file is a good idea.
--- Code: ---+struct TempFrameDeleter
+{
+ void operator()(wxFrame* frame)
+ {
+ frame->Destroy();
+ }
+};
+
+using TempFrameUniquePtr = std::unique_ptr<wxFrame, TempFrameDeleter>;
--- End code ---
EDIT2:
Follow this page: c++ - Simple custom deleter lambda supplied to std::unique_ptr: why use by-reference capture default ([&]) over no-capture ([])? - Stack Overflow
This is more compact way of define the unique_ptr, OK with below code?
--- Code: --- std::unique_ptr<wxFrame, std::function<void(wxFrame*)>> tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
[&](wxFrame* frame){ frame->Destroy(); }); // deleter
--- End code ---
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version