From 9d7f0c5f7e522a0916526bf1ea3e59c4e6573501 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Mon, 14 Oct 2013 15:07:44 +0000
Subject: [PATCH] Avoid spurious errors when getting wxChoice or wxListBox
 client data in wxMSW.

Even after the changes of r70415, we could still report an error when the
item client data was set to -1 (== CB_ERR) as checking for GetLastError() was
not enough, we need to also ensure that the last error is reset before trying
to get the client data.

Also apply the same fix to wxListBox and add the tests verifying that this
does work correctly.

Closes #13883.

Also fix wxListBox::GetClientData().

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@74994 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775
---
 src/msw/choice.cpp                   |  9 +++++++++
 src/msw/listbox.cpp                  |  4 ++++
 tests/controls/itemcontainertest.cpp | 12 ++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/src/msw/choice.cpp b/src/msw/choice.cpp
index 26e4a8e135..6699e1e95b 100644
--- a/src/msw/choice.cpp
+++ b/src/msw/choice.cpp
@@ -436,7 +436,16 @@ void wxChoice::DoSetItemClientData(unsigned int n, void* clientData)
 
 void* wxChoice::DoGetItemClientData(unsigned int n) const
 {
+    // Before using GetLastError() below, ensure that we don't have a stale
+    // error code from a previous API call as CB_GETITEMDATA doesn't reset it
+    // in case of success, it only sets it if an error occurs.
+    SetLastError(ERROR_SUCCESS);
+
     LPARAM rc = SendMessage(GetHwnd(), CB_GETITEMDATA, n, 0);
+
+    // Notice that we must call GetLastError() to distinguish between a real
+    // error and successfully retrieving a previously stored client data value
+    // of CB_ERR (-1).
     if ( rc == CB_ERR && GetLastError() != ERROR_SUCCESS )
     {
         wxLogLastError(wxT("CB_GETITEMDATA"));
diff --git a/src/msw/listbox.cpp b/src/msw/listbox.cpp
index e0f34e3637..4a452a16bf 100644
--- a/src/msw/listbox.cpp
+++ b/src/msw/listbox.cpp
@@ -304,6 +304,10 @@ bool wxListBox::IsSelected(int N) const
 
 void *wxListBox::DoGetItemClientData(unsigned int n) const
 {
+    // This is done here for the same reasons as in wxChoice method with the
+    // same name.
+    SetLastError(ERROR_SUCCESS);
+
     LPARAM rc = SendMessage(GetHwnd(), LB_GETITEMDATA, n, 0);
     if ( rc == LB_ERR && GetLastError() != ERROR_SUCCESS )
     {
diff --git a/tests/controls/itemcontainertest.cpp b/tests/controls/itemcontainertest.cpp
index aa9df2a8e7..3b5c003110 100644
--- a/tests/controls/itemcontainertest.cpp
+++ b/tests/controls/itemcontainertest.cpp
@@ -200,6 +200,18 @@ void ItemContainerTestCase::VoidData()
 
     WX_ASSERT_FAILS_WITH_ASSERT( container->SetClientData((unsigned)-1, NULL) );
     WX_ASSERT_FAILS_WITH_ASSERT( container->SetClientData(12345, NULL) );
+
+    // wxMSW used to hace problems retrieving the client data of -1 from a few
+    // standard controls, especially if the last error was set before doing it,
+    // so test for this specially.
+    const wxUIntPtr minus1 = static_cast<wxUIntPtr>(-1);
+    container->Append("item -1", wxUIntToPtr(minus1));
+
+#ifdef __WINDOWS__
+    ::SetLastError(ERROR_INVALID_DATA);
+#endif
+
+    CPPUNIT_ASSERT_EQUAL( minus1, wxPtrToUInt(container->GetClientData(3)) );
 }
 
 void ItemContainerTestCase::Set()