Check index in wxItemContainer methods working with client data.

The test for index validity should be done by the base class public methods
themselves so that the protected methods in the derived classes don't need to
do it because this allows to have the check in one place only and not in every
port-specific derived class and also because a protected method can reasonably
expect to be called with already validated parameters.

This makes it unnecessary to perform the same check in many derived classes
and fixes the problem with those that forgot to check for item validity at all
before (like wxGTK wxChoice).

Also add a unit test checking for the correct behaviour. Unfortunately we
don't have any way to test for the precise assert being triggered so the test
passed for wxGTK wxChoice even before in debug builds because the expected
assert was raised by wxArray::Item() but the code crashed in release build --
whereas now it doesn't any more.

Closes #12858.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@66664 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/src/common/ctrlsub.cpp b/src/common/ctrlsub.cpp
index 68d4b1a5916150f05b753abca0a2c6d77307e0f2..388af3e6ffb77501e18589e4ae79c5aed5418838 100644 (file)
--- a/src/common/ctrlsub.cpp
+++ b/src/common/ctrlsub.cpp
@@ -165,6 +165,8 @@ void wxItemContainer::SetClientObject(unsigned int n, wxClientData *data)
     wxASSERT_MSG( !HasClientUntypedData(),
                   wxT("can't have both object and void client data") );
 
+    wxCHECK_RET( IsValid(n), "Invalid index passed to SetClientObject()" );
+
     if ( HasClientObjectData() )
     {
         wxClientData * clientDataOld
@@ -188,6 +190,9 @@ wxClientData *wxItemContainer::GetClientObject(unsigned int n) const
     wxCHECK_MSG( HasClientObjectData(), NULL,
                   wxT("this window doesn't have object client data") );
 
+    wxCHECK_MSG( IsValid(n), NULL,
+                 "Invalid index passed to GetClientObject()" );
+
     return static_cast<wxClientData *>(DoGetItemClientData(n));
 }
 
@@ -214,6 +219,8 @@ void wxItemContainer::SetClientData(unsigned int n, void *data)
     wxASSERT_MSG( HasClientUntypedData(),
                   wxT("can't have both object and void client data") );
 
+    wxCHECK_RET( IsValid(n), "Invalid index passed to SetClientData()" );
+
     DoSetItemClientData(n, data);
 }
 
@@ -222,6 +229,9 @@ void *wxItemContainer::GetClientData(unsigned int n) const
     wxCHECK_MSG( HasClientUntypedData(), NULL,
                   wxT("this window doesn't have void client data") );
 
+    wxCHECK_MSG( IsValid(n), NULL,
+                 "Invalid index passed to GetClientData()" );
+
     return DoGetItemClientData(n);
 }
 

diff --git a/src/gtk/listbox.cpp b/src/gtk/listbox.cpp
index 06c62c2ff566cbd26dc5bbf49dbe4435aa56c15e..9a406184bd1db7f34d1be1681ceabba7b540514c 100644 (file)
--- a/src/gtk/listbox.cpp
+++ b/src/gtk/listbox.cpp
@@ -578,9 +578,6 @@ void wxListBox::GTKSetItem(GtkTreeIter& iter, const GtkTreeEntry *entry)
 
 void* wxListBox::DoGetItemClientData(unsigned int n) const
 {
-    wxCHECK_MSG( IsValid(n), NULL,
-                 wxT("Invalid index passed to GetItemClientData") );
-
     wxGtkObject<GtkTreeEntry> entry(GTKGetEntry(n));
     wxCHECK_MSG(entry, NULL, wxT("could not get entry"));
 
@@ -589,9 +586,6 @@ void* wxListBox::DoGetItemClientData(unsigned int n) const
 
 void wxListBox::DoSetItemClientData(unsigned int n, void* clientData)
 {
-    wxCHECK_RET( IsValid(n),
-                 wxT("Invalid index passed to SetItemClientData") );
-
     wxGtkObject<GtkTreeEntry> entry(GTKGetEntry(n));
     wxCHECK_RET(entry, wxT("could not get entry"));
 

diff --git a/src/msw/listbox.cpp b/src/msw/listbox.cpp
index 69281add2b8f7c109a8204041fd30dd2ee868dce..69565096f7b90a0fd9dc8c3cea7892829375b4fc 100644 (file)
--- a/src/msw/listbox.cpp
+++ b/src/msw/listbox.cpp
@@ -306,17 +306,11 @@ bool wxListBox::IsSelected(int N) const
 
 void *wxListBox::DoGetItemClientData(unsigned int n) const
 {
-    wxCHECK_MSG( IsValid(n), NULL,
-                 wxT("invalid index in wxListBox::GetClientData") );
-
     return (void *)SendMessage(GetHwnd(), LB_GETITEMDATA, n, 0);
 }
 
 void wxListBox::DoSetItemClientData(unsigned int n, void *clientData)
 {
-    wxCHECK_RET( IsValid(n),
-                 wxT("invalid index in wxListBox::SetClientData") );
-
     if ( ListBox_SetItemData(GetHwnd(), n, clientData) == LB_ERR )
     {
         wxLogDebug(wxT("LB_SETITEMDATA failed"));

diff --git a/src/osx/choice_osx.cpp b/src/osx/choice_osx.cpp
index e3d6cdab4827dd8ed3c7db863936d96e72fcb244..28af5cdf2c0dcb8e3b8c40a3693ee1745ac5b632 100644 (file)
--- a/src/osx/choice_osx.cpp
+++ b/src/osx/choice_osx.cpp
@@ -219,15 +219,11 @@ wxString wxChoice::GetString(unsigned int n) const
 // ----------------------------------------------------------------------------
 void wxChoice::DoSetItemClientData(unsigned int n, void* clientData)
 {
-    wxCHECK_RET( IsValid(n), wxT("wxChoice::DoSetItemClientData: invalid index") );
-
     m_datas[n] = (char*)clientData ;
 }
 
 void * wxChoice::DoGetItemClientData(unsigned int n) const
 {
-    wxCHECK_MSG( IsValid(n), NULL, wxT("wxChoice::DoGetClientData: invalid index") );
-
     return (void *)m_datas[n];
 }
 

diff --git a/src/osx/combobox_osx.cpp b/src/osx/combobox_osx.cpp
index 8e814f8ebaa2b34dc7326acb7e11f0a92909b275..bf092f3772fdfacb043e0bbe96e6ae5fc4c3e95e 100644 (file)
--- a/src/osx/combobox_osx.cpp
+++ b/src/osx/combobox_osx.cpp
@@ -120,15 +120,11 @@ int wxComboBox::DoInsertItems(const wxArrayStringsAdapter& items,
 // ----------------------------------------------------------------------------
 void wxComboBox::DoSetItemClientData(unsigned int n, void* clientData)
 {
-    wxCHECK_RET( IsValid(n), "invalid index" );
-
     m_datas[n] = (char*)clientData ;
 }
 
 void * wxComboBox::DoGetItemClientData(unsigned int n) const
 {
-    wxCHECK_MSG( IsValid(n), NULL, "invalid index" );
-
     return (void *)m_datas[n];
 }
 

diff --git a/tests/controls/itemcontainertest.cpp b/tests/controls/itemcontainertest.cpp
index ab8b9807fbd75e3768168c3e83f2602658ab3d9f..c470749098272ed1fb5cac3a308eb243631e52f6 100644 (file)
--- a/tests/controls/itemcontainertest.cpp
+++ b/tests/controls/itemcontainertest.cpp
@@ -170,6 +170,9 @@ void ItemContainerTestCase::ClientData()
 
     CPPUNIT_ASSERT_EQUAL(static_cast<wxClientData*>(item2data),
                          container->GetClientObject(2));
+
+    WX_ASSERT_FAILS_WITH_ASSERT( container->SetClientObject(-1, item0data) );
+    WX_ASSERT_FAILS_WITH_ASSERT( container->SetClientObject(12345, item0data) );
 }
 
 void ItemContainerTestCase::VoidData()
@@ -195,6 +198,9 @@ void ItemContainerTestCase::VoidData()
     container->Insert("item 2", 2, item2);
 
     CPPUNIT_ASSERT_EQUAL(item2, container->GetClientData(2));
+
+    WX_ASSERT_FAILS_WITH_ASSERT( container->SetClientData(-1, NULL) );
+    WX_ASSERT_FAILS_WITH_ASSERT( container->SetClientData(12345, NULL) );
 }
 
 void ItemContainerTestCase::Set()