From 5c6bce627b289d2c935f3a73ad0977f880d8707d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 26 Nov 2022 19:52:09 +0100 Subject: [PATCH] Check wxImageList validity in all of its methods Some of wxImageList methods asserted when called on an invalid image list while others just failed silently. Assert in all of them now for consistency and to help detecting problems in the code using invalid wxImageList objects. Extend the documentation and the tests. --- docs/changes.txt | 5 +++++ interface/wx/imaglist.h | 4 ++++ src/generic/imaglist.cpp | 9 +++++++-- src/msw/imaglist.cpp | 16 ++++++++++++++++ tests/graphics/imagelist.cpp | 22 ++++++++++++++++------ 5 files changed, 48 insertions(+), 8 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index d1118c9390..5373966573 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -41,6 +41,11 @@ Changes in behaviour not resulting in compilation errors even if the grid is currently too small to show all of them or if there are no unfrozen rows/columns remaining. +- Calling wxImageList methods on an invalid wxImageList object now consistently + results in assert failures instead of just failing silently. To avoid these + asserts, make sure that wxImageList is created with a valid size before + operating on it. + Changes in behaviour which may result in build errors ----------------------------------------------------- diff --git a/interface/wx/imaglist.h b/interface/wx/imaglist.h index 55d3a68826..720690813b 100644 --- a/interface/wx/imaglist.h +++ b/interface/wx/imaglist.h @@ -48,6 +48,10 @@ class wxImageList : public wxObject public: /** Default ctor. + + Note that the object created using the default ctor is invalid and + calling any methods other than Create() on it will result in an + assertion failure. */ wxImageList(); diff --git a/src/generic/imaglist.cpp b/src/generic/imaglist.cpp index 20dc318daa..4a0df58511 100644 --- a/src/generic/imaglist.cpp +++ b/src/generic/imaglist.cpp @@ -112,8 +112,7 @@ wxBitmap wxGenericImageList::GetImageListBitmap(const wxBitmap& bitmap) const int wxGenericImageList::Add( const wxBitmap &bitmap ) { // Cannot add image to invalid list - if ( m_size == wxSize(0, 0) ) - return -1; + wxCHECK_MSG( m_size != wxSize(0, 0), -1, "Invalid image list" ); // We use the logical size here as image list images size is specified in // logical pixels, just as window coordinates and sizes are. @@ -160,6 +159,8 @@ int wxGenericImageList::Add( const wxBitmap& bitmap, const wxColour& maskColour const wxBitmap *wxGenericImageList::DoGetPtr( int index ) const { + wxCHECK_MSG( m_size != wxSize(0, 0), nullptr, "Invalid image list" ); + if ( index < 0 || (size_t)index >= m_images.size() ) return nullptr; @@ -208,6 +209,8 @@ wxGenericImageList::Replace(int index, bool wxGenericImageList::Remove( int index ) { + wxCHECK_MSG( m_size != wxSize(0, 0), false, "Invalid image list" ); + if ( index < 0 || (size_t)index >= m_images.size() ) return false; @@ -218,6 +221,8 @@ bool wxGenericImageList::Remove( int index ) bool wxGenericImageList::RemoveAll() { + wxCHECK_MSG( m_size != wxSize(0, 0), false, "Invalid image list" ); + m_images.clear(); return true; diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index d326621bec..e3ecff5ef7 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -293,6 +293,8 @@ wxImageList::GetImageListBitmaps(wxMSWBitmaps& bitmaps, // 'bitmap' and 'mask'. int wxImageList::Add(const wxBitmap& bitmap, const wxBitmap& mask) { + wxASSERT_MSG( m_hImageList, wxT("invalid image list") ); + wxMSWBitmaps bitmaps; GetImageListBitmaps(bitmaps, bitmap, mask); @@ -310,6 +312,8 @@ int wxImageList::Add(const wxBitmap& bitmap, const wxBitmap& mask) // 'bitmap'. int wxImageList::Add(const wxBitmap& bitmap, const wxColour& maskColour) { + wxASSERT_MSG( m_hImageList, wxT("invalid image list") ); + wxMSWBitmaps bitmaps; wxMask mask(bitmap, maskColour); GetImageListBitmaps(bitmaps, bitmap, mask.GetBitmap()); @@ -342,6 +346,8 @@ bool wxImageList::Replace(int index, const wxBitmap& bitmap, const wxBitmap& mask) { + wxASSERT_MSG( m_hImageList, wxT("invalid image list") ); + wxMSWBitmaps bitmaps; GetImageListBitmaps(bitmaps, bitmap, mask); @@ -365,6 +371,8 @@ bool wxImageList::Replace(int i, const wxIcon& icon) // Removes the image at the given index. bool wxImageList::Remove(int index) { + wxASSERT_MSG( m_hImageList, wxT("invalid image list") ); + bool ok = index >= 0 && ImageList_Remove(GetHImageList(), index) != FALSE; if ( !ok ) { @@ -377,6 +385,8 @@ bool wxImageList::Remove(int index) // Remove all images bool wxImageList::RemoveAll() { + wxASSERT_MSG( m_hImageList, wxT("invalid image list") ); + // don't use ImageList_RemoveAll() because mingw32 headers don't have it bool ok = ImageList_Remove(GetHImageList(), -1) != FALSE; if ( !ok ) @@ -397,6 +407,8 @@ bool wxImageList::Draw(int index, int flags, bool solidBackground) { + wxASSERT_MSG( m_hImageList, wxT("invalid image list") ); + wxDCImpl *impl = dc.GetImpl(); wxMSWDCImpl *msw_impl = wxDynamicCast( impl, wxMSWDCImpl ); if (!msw_impl) @@ -445,6 +457,8 @@ bool wxImageList::Draw(int index, // Get the bitmap wxBitmap wxImageList::GetBitmap(int index) const { + wxASSERT_MSG( m_hImageList, wxT("invalid image list") ); + int bmp_width = 0, bmp_height = 0; GetSize(index, bmp_width, bmp_height); @@ -508,6 +522,8 @@ wxBitmap wxImageList::GetBitmap(int index) const // Get the icon wxIcon wxImageList::GetIcon(int index) const { + wxASSERT_MSG( m_hImageList, wxT("invalid image list") ); + HICON hIcon = ImageList_ExtractIcon(0, GetHImageList(), index); if (hIcon) { diff --git a/tests/graphics/imagelist.cpp b/tests/graphics/imagelist.cpp index bbe0b564d5..4e564ff91a 100644 --- a/tests/graphics/imagelist.cpp +++ b/tests/graphics/imagelist.cpp @@ -609,11 +609,11 @@ TEST_CASE("ImageList:NegativeTests", "[imagelist][negative]") CHECK(h == 0); #endif - int idx = il.Add(bmp); - CHECK(idx == -1); #ifdef __WXDEBUG__ + CHECK_THROWS(il.Add(bmp)); REQUIRE_THROWS(il.GetImageCount()); #else + CHECK(il.Add(bmp) == -1); CHECK(il.GetImageCount() == 0); #endif } @@ -644,23 +644,33 @@ TEST_CASE("ImageList:NegativeTests", "[imagelist][negative]") CHECK(h == 0); #endif - int idx = il.Add(bmp); - CHECK(idx == -1); #ifdef __WXDEBUG__ + CHECK_THROWS(il.Add(bmp)); REQUIRE_THROWS(il.GetImageCount()); #else + CHECK(il.Add(bmp) == -1); CHECK(il.GetImageCount() == 0); #endif - ok = il.Replace(0, bmp); - CHECK_FALSE(ok); #ifdef __WXDEBUG__ + CHECK_THROWS(il.Replace(0, bmp)); REQUIRE_THROWS(il.GetImageCount()); #else + CHECK_FALSE(il.Replace(0, bmp)); CHECK(il.GetImageCount() == 0); #endif } + SECTION("Add to invalid image list") + { + wxImageList il; +#ifdef __WXDEBUG__ + CHECK_THROWS( il.Add(bmp) ); +#else + CHECK( il.Add(bmp) == -1 ); +#endif + } + SECTION("Invalid Get/Replace/Remove indices") { wxImageList il(32, 32, false);