From 98aaeadf54882c7f09eac0804de9b808b2ca30ce Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 17 Jan 2021 00:10:23 +0100 Subject: [PATCH 1/4] Assert in wxMSW wxChoice::GetString() if the index is invalid Do it for consistency with SetString() and other controls. --- src/msw/choice.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/msw/choice.cpp b/src/msw/choice.cpp index 8f66f7877a..9a760f56d8 100644 --- a/src/msw/choice.cpp +++ b/src/msw/choice.cpp @@ -362,10 +362,13 @@ void wxChoice::SetString(unsigned int n, const wxString& s) wxString wxChoice::GetString(unsigned int n) const { - int len = (int)::SendMessage(GetHwnd(), CB_GETLBTEXTLEN, n, 0); + const int len = (int)::SendMessage(GetHwnd(), CB_GETLBTEXTLEN, n, 0); wxString str; - if ( len != CB_ERR && len > 0 ) + + wxCHECK_MSG( len != CB_ERR, str, wxS("Invalid index") ); + + if ( len > 0 ) { if ( ::SendMessage ( From 910dfbc0101e36fbe660db5f4342a1a488ae875d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 17 Jan 2021 00:20:51 +0100 Subject: [PATCH 2/4] Assert in wxGTK wxChoice::GetString() if the index is invalid Similar to the previous commit for wxMSW. This commit is best viewed ignoring whitespace-only changes. --- src/gtk/choice.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/gtk/choice.cpp b/src/gtk/choice.cpp index 9c6ebc9039..ff0b6ea224 100644 --- a/src/gtk/choice.cpp +++ b/src/gtk/choice.cpp @@ -278,21 +278,20 @@ wxString wxChoice::GetString(unsigned int n) const { wxCHECK_MSG( m_widget != NULL, wxEmptyString, wxT("invalid control") ); - wxString str; - GtkComboBox* combobox = GTK_COMBO_BOX( m_widget ); GtkTreeModel *model = gtk_combo_box_get_model( combobox ); GtkTreeIter iter; - if (gtk_tree_model_iter_nth_child (model, &iter, NULL, n)) + if (!gtk_tree_model_iter_nth_child (model, &iter, NULL, n)) { - GValue value = G_VALUE_INIT; - gtk_tree_model_get_value( model, &iter, m_stringCellIndex, &value ); - wxString tmp = wxGTK_CONV_BACK( g_value_get_string( &value ) ); - g_value_unset( &value ); - return tmp; + wxFAIL_MSG( "invalid index" ); + return wxString(); } - return str; + GValue value = G_VALUE_INIT; + gtk_tree_model_get_value( model, &iter, m_stringCellIndex, &value ); + wxString tmp = wxGTK_CONV_BACK( g_value_get_string( &value ) ); + g_value_unset( &value ); + return tmp; } unsigned int wxChoice::GetCount() const From 1ddf2ee303d8f35ce21677eed79acc18fcd4e2c8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 17 Jan 2021 00:09:11 +0100 Subject: [PATCH 3/4] Check that calling GetString() with invalid index asserts This is the behaviour that would be normally expected and now really happens for all the controls, including wxChoice. --- tests/controls/itemcontainertest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/controls/itemcontainertest.cpp b/tests/controls/itemcontainertest.cpp index ffc49d0197..7c28b1a0f9 100644 --- a/tests/controls/itemcontainertest.cpp +++ b/tests/controls/itemcontainertest.cpp @@ -71,6 +71,7 @@ void ItemContainerTestCase::Count() wxItemContainer * const container = GetContainer(); CPPUNIT_ASSERT(container->IsEmpty()); + WX_ASSERT_FAILS_WITH_ASSERT( container->GetString(0) ); wxArrayString testitems; testitems.Add("item 0"); @@ -95,6 +96,7 @@ void ItemContainerTestCase::Count() container->Insert(testitems, 1); CPPUNIT_ASSERT_EQUAL(5, container->GetCount()); + WX_ASSERT_FAILS_WITH_ASSERT( container->GetString(10) ); } void ItemContainerTestCase::ItemSelection() From 4a7b6d7e8fcf68d470ce171622b660fcdc6db34b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 17 Jan 2021 00:08:30 +0100 Subject: [PATCH 4/4] Document that index must be valid in wxChoice::GetString() This is now the case in all ports, and not just in wxOSX, so document this behaviour and also document that it has changed compared to 3.0. --- docs/changes.txt | 2 ++ interface/wx/ctrlsub.h | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 43013289be..a80274f466 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -107,6 +107,8 @@ Changes in behaviour not resulting in compilation errors - wxGetInstallPrefix() now returns wxString. +- wxChoice::GetString() now consistently asserts when passed an invalid index. + Changes in behaviour which may result in build errors ----------------------------------------------------- diff --git a/interface/wx/ctrlsub.h b/interface/wx/ctrlsub.h index 68fb7f8980..e246e6a26c 100644 --- a/interface/wx/ctrlsub.h +++ b/interface/wx/ctrlsub.h @@ -51,11 +51,14 @@ public: /** Returns the label of the item with the given index. + The index must be valid, i.e. less than the value returned by + GetCount(), otherwise an assert is triggered. Notably, this function + can't be called if the control is empty. + @param n The zero-based index. - @return The label of the item or an empty string if the position was - invalid. + @return The label of the item. */ virtual wxString GetString(unsigned int n) const = 0;