From: Vadim Zeitlin Date: Mon, 10 Jan 2011 12:00:54 +0000 (+0000) Subject: Check index in wxItemContainer methods working with client data. X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/8584b0e64b273273bad122d28b10176bd5a3bc84?ds=inline 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 68d4b1a591..388af3e6ff 100644 --- 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(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 06c62c2ff5..9a406184bd 100644 --- 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 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 entry(GTKGetEntry(n)); wxCHECK_RET(entry, wxT("could not get entry")); diff --git a/src/msw/listbox.cpp b/src/msw/listbox.cpp index 69281add2b..69565096f7 100644 --- 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 e3d6cdab48..28af5cdf2c 100644 --- 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 8e814f8eba..bf092f3772 100644 --- 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 ab8b9807fb..c470749098 100644 --- a/tests/controls/itemcontainertest.cpp +++ b/tests/controls/itemcontainertest.cpp @@ -170,6 +170,9 @@ void ItemContainerTestCase::ClientData() CPPUNIT_ASSERT_EQUAL(static_cast(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()