From 46bfb43d21668b609847e2e9e651f1701d6c8937 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Sat, 18 Feb 2023 16:04:21 +0000
Subject: [PATCH] Centralize index checking in wxSizer code

Add GetChildNode() helper and give more information in the assert
message generated there if the index is invalid, notably the value of
the index itself.

No real changes, this is just a small code simplification and quality of
debugging improvement.
---
 include/wx/sizer.h   |  3 ++
 src/common/sizer.cpp | 65 ++++++++++++++++++++++++++------------------
 2 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/include/wx/sizer.h b/include/wx/sizer.h
index 31aec3b61d..d026fbd838 100644
--- a/include/wx/sizer.h
+++ b/include/wx/sizer.h
@@ -770,6 +770,9 @@ protected:
     virtual wxSizerItem* DoInsert(size_t index, wxSizerItem *item);
 
 private:
+    // Get the child item with the given index and assert if there is none.
+    wxSizerItemList::compatibility_iterator GetChildNode(size_t index) const;
+
     wxDECLARE_CLASS(wxSizer);
 };
 
diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp
index 25b68f0efb..539965c45f 100644
--- a/src/common/sizer.cpp
+++ b/src/common/sizer.cpp
@@ -858,6 +858,22 @@ wxSizerItem* wxSizer::DoInsert( size_t index, wxSizerItem *item )
     return guard.Release();
 }
 
+wxSizerItemList::compatibility_iterator wxSizer::GetChildNode(size_t index) const
+{
+    wxCHECK_MSG
+    (
+        index < m_children.GetCount(),
+        {},
+        wxString::Format
+        (
+            "Invalid sizer item child index %zu, sizer has only %zu elements.",
+            index, m_children.GetCount()
+        )
+    );
+
+    return m_children.Item( index );
+}
+
 void wxSizer::SetContainingWindow(wxWindow *win)
 {
     if ( win == m_containingWindow )
@@ -910,13 +926,11 @@ bool wxSizer::Remove( wxSizer *sizer )
 
 bool wxSizer::Remove( int index )
 {
-    wxCHECK_MSG( index >= 0 && (size_t)index < m_children.GetCount(),
-                 false,
-                 wxT("Remove index is out of range") );
+    wxCHECK_MSG( index >= 0, false, wxT("Index must be positive") );
 
-    wxSizerItemList::compatibility_iterator node = m_children.Item( index );
-
-    wxCHECK_MSG( node, false, wxT("Failed to find child node") );
+    const auto node = GetChildNode(static_cast<size_t>(index));
+    if ( !node )
+        return false;
 
     delete node->GetData();
     m_children.Erase( node );
@@ -969,13 +983,11 @@ bool wxSizer::Detach( wxWindow *window )
 
 bool wxSizer::Detach( int index )
 {
-    wxCHECK_MSG( index >= 0 && (size_t)index < m_children.GetCount(),
-                 false,
-                 wxT("Detach index is out of range") );
+    wxCHECK_MSG( index >= 0, false, wxT("Index must be positive") );
 
-    wxSizerItemList::compatibility_iterator node = m_children.Item( index );
-
-    wxCHECK_MSG( node, false, wxT("Failed to find child node") );
+    const auto node = GetChildNode(static_cast<size_t>(index));
+    if ( !node )
+        return false;
 
     wxSizerItem *item = node->GetData();
 
@@ -1034,12 +1046,11 @@ bool wxSizer::Replace( wxSizer *oldsz, wxSizer *newsz, bool recursive )
 
 bool wxSizer::Replace( size_t old, wxSizerItem *newitem )
 {
-    wxCHECK_MSG( old < m_children.GetCount(), false, wxT("Replace index is out of range") );
     wxCHECK_MSG( newitem, false, wxT("Replacing with null item") );
 
-    wxSizerItemList::compatibility_iterator node = m_children.Item( old );
-
-    wxCHECK_MSG( node, false, wxT("Failed to find child node") );
+    auto node = GetChildNode(old);
+    if ( !node )
+        return false;
 
     wxSizerItem *item = node->GetData();
     node->SetData(newitem);
@@ -1282,9 +1293,9 @@ bool wxSizer::DoSetItemMinSize( wxSizer *sizer, int width, int height )
 
 bool wxSizer::DoSetItemMinSize( size_t index, int width, int height )
 {
-    wxSizerItemList::compatibility_iterator node = m_children.Item( index );
-
-    wxCHECK_MSG( node, false, wxT("Failed to find child node") );
+    const auto node = GetChildNode(index);
+    if ( !node )
+        return false;
 
     wxSizerItem     *item = node->GetData();
 
@@ -1346,11 +1357,11 @@ wxSizerItem* wxSizer::GetItem( wxSizer *sizer, bool recursive )
 
 wxSizerItem* wxSizer::GetItem( size_t index )
 {
-    wxCHECK_MSG( index < m_children.GetCount(),
-                 nullptr,
-                 wxT("GetItem index is out of range") );
+    const auto node = GetChildNode(index);
+    if ( !node )
+        return nullptr;
 
-    return m_children.Item( index )->GetData();
+    return node->GetData();
 }
 
 wxSizerItem* wxSizer::GetItemById( int id, bool recursive )
@@ -1465,11 +1476,11 @@ bool wxSizer::IsShown( wxSizer *sizer ) const
 
 bool wxSizer::IsShown( size_t index ) const
 {
-    wxCHECK_MSG( index < m_children.GetCount(),
-                 false,
-                 wxT("IsShown index is out of range") );
+    const auto node = GetChildNode(index);
+    if ( !node )
+        return false;
 
-    return m_children.Item( index )->GetData()->IsShown();
+    return node->GetData()->IsShown();
 }