Fix some off by one calculations in the comboboxex functions, and

handle an out of range positive index the same as windows + unit test
case.

dlls/comctl32/comboex.c

@@ -473,7 +473,7 @@ static CBE_ITEMDATA * COMBOEX_FindItem(COMBOEX_INFO *infoPtr, INT index)
     CBE_ITEMDATA *item;
     INT i;
 
-    if ((index > infoPtr->nb_items) || (index < -1))
+    if ((index >= infoPtr->nb_items) || (index < -1))
 	return 0;
     if (index == -1)
 	return infoPtr->edit;
@@ -509,7 +509,7 @@ static INT COMBOEX_DeleteItem (COMBOEX_INFO *infoPtr, INT index)
     TRACE("(index=%d)\n", index);
 
     /* if item number requested does not exist then return failure */
-    if ((index > infoPtr->nb_items) || (index < 0)) return CB_ERR;
+    if ((index >= infoPtr->nb_items) || (index < 0)) return CB_ERR;
     if (!(item = COMBOEX_FindItem(infoPtr, index))) return CB_ERR;
 
     /* doing this will result in WM_DELETEITEM being issued */
@@ -527,7 +527,7 @@ static BOOL COMBOEX_GetItemW (COMBOEX_INFO *infoPtr, COMBOBOXEXITEMW *cit)
     TRACE("(...)\n");
 
     /* if item number requested does not exist then return failure */
-    if ((index > infoPtr->nb_items) || (index < -1)) return FALSE;
+    if ((index >= infoPtr->nb_items) || (index < -1)) return FALSE;
 
     /* if the item is the edit control and there is no edit control, skip */
     if ((index == -1) && !COMBOEX_HasEdit(infoPtr)) return FALSE;
@@ -587,7 +587,7 @@ static INT COMBOEX_InsertItemW (COMBOEX_INFO *infoPtr, COMBOBOXEXITEMW *cit)
     /* get real index of item to insert */
     index = cit->iItem;
     if (index == -1) index = infoPtr->nb_items;
-    if (index > infoPtr->nb_items) index = infoPtr->nb_items;
+    if (index > infoPtr->nb_items) return -1;
 
     /* get zero-filled space and chain it in */
     if(!(item = (CBE_ITEMDATA *)Alloc (sizeof(*item)))) return -1;
@@ -740,7 +740,7 @@ static BOOL COMBOEX_SetItemW (COMBOEX_INFO *infoPtr, COMBOBOXEXITEMW *cit)
     if (TRACE_ON(comboex)) COMBOEX_DumpInput (cit);
 
     /* if item number requested does not exist then return failure */
-    if ((index > infoPtr->nb_items) || (index < -1)) return FALSE;
+    if ((index >= infoPtr->nb_items) || (index < -1)) return FALSE;
 
     /* if the item is the edit control and there is no edit control, skip */
     if ((index == -1) && !COMBOEX_HasEdit(infoPtr)) return FALSE;

dlls/comctl32/tests/.cvsignore

@@ -1,4 +1,5 @@
 Makefile
+comboex.ok
 dpa.ok
 imagelist.ok
 mru.ok

dlls/comctl32/tests/Makefile.in

@@ -6,6 +6,7 @@ TESTDLL   = comctl32.dll
 IMPORTS   = comctl32 ole32 shlwapi user32 gdi32 advapi32 kernel32
 
 CTESTS = \
+	comboex.c \
 	dpa.c \
 	imagelist.c \
 	mru.c \

dlls/comctl32/tests/comboex.c

@@ -0,0 +1,236 @@
+/* Unit test suite for comboex control.
+ *
+ * Copyright 2005 Jason Edmeades
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <assert.h>
+#include <windows.h>
+#include <commctrl.h>
+
+#include "wine/test.h"
+
+static HWND hComboExParentWnd;
+static HINSTANCE hMainHinst;
+static char ComboExTestClass[] = "ComboExTestClass";
+
+#define MAX_CHARS 100
+static char *textBuffer = NULL;
+
+static HWND createComboEx(DWORD style) {
+   return CreateWindowExA(0, WC_COMBOBOXEXA, NULL, style, 0, 0, 300, 300,
+            hComboExParentWnd, NULL, hMainHinst, NULL);
+}
+
+static LONG addItem(HWND cbex, int idx, LPTSTR text) {
+    COMBOBOXEXITEM cbexItem;
+    memset(&cbexItem, 0x00, sizeof(cbexItem));
+    cbexItem.mask = CBEIF_TEXT;
+    cbexItem.iItem = idx;
+    cbexItem.pszText    = text;
+    cbexItem.cchTextMax = 0;
+    return (LONG)SendMessage(cbex, CBEM_INSERTITEM, 0,(LPARAM)&cbexItem);
+}
+
+static LONG setItem(HWND cbex, int idx, LPTSTR text) {
+    COMBOBOXEXITEM cbexItem;
+    memset(&cbexItem, 0x00, sizeof(cbexItem));
+    cbexItem.mask = CBEIF_TEXT;
+    cbexItem.iItem = idx;
+    cbexItem.pszText    = text;
+    cbexItem.cchTextMax = 0;
+    return (LONG)SendMessage(cbex, CBEM_SETITEM, 0,(LPARAM)&cbexItem);
+}
+
+static LONG delItem(HWND cbex, int idx) {
+    return (LONG)SendMessage(cbex, CBEM_DELETEITEM, (LPARAM)idx, 0);
+}
+
+static LONG getItem(HWND cbex, int idx, COMBOBOXEXITEM *cbItem) {
+    memset(cbItem, 0x00, sizeof(COMBOBOXEXITEM));
+    cbItem->mask = CBEIF_TEXT;
+    cbItem->pszText      = textBuffer;
+    cbItem->iItem        = idx;
+    cbItem->cchTextMax   = 100;
+    return (LONG)SendMessage(cbex, CBEM_GETITEM, 0, (LPARAM)cbItem);
+}
+
+static void test_comboboxex() {
+    HWND myHwnd = 0;
+    LONG res = -1;
+    COMBOBOXEXITEM cbexItem;
+
+    #define FIRST_ITEM       "First Item"
+    #define SECOND_ITEM      "Second Item"
+    #define THIRD_ITEM       "Third Item"
+    #define MIDDLE_ITEM      "Between First and Second Items"
+    #define REPLACEMENT_ITEM "Between First and Second Items"
+
+    /* Allocate space for result */
+    textBuffer = malloc(MAX_CHARS);
+
+    /* Basic comboboxex test */
+    myHwnd = createComboEx(WS_BORDER | WS_VISIBLE | WS_CHILD | CBS_DROPDOWN);
+
+    /* Add items onto the end of the combobox */
+    res = addItem(myHwnd, -1, FIRST_ITEM);
+    ok(res == 0, "Adding simple item failed (%ld)\n", res);
+    res = addItem(myHwnd, -1, SECOND_ITEM);
+    ok(res == 1, "Adding simple item failed (%ld)\n", res);
+    res = addItem(myHwnd, 2, THIRD_ITEM);
+    ok(res == 2, "Adding simple item failed (%ld)\n", res);
+    res = addItem(myHwnd, 1, MIDDLE_ITEM);
+    ok(res == 1, "Inserting simple item failed (%ld)\n", res);
+
+    /* Add an item completely out of range */
+    res = addItem(myHwnd, 99, "Out Of Range Item");
+    ok(res == -1, "Adding using out of range index worked unexpectedly (%ld)\n", res);
+    res = addItem(myHwnd, 5, "Out Of Range Item");
+    ok(res == -1, "Adding using out of range index worked unexpectedly (%ld)\n", res);
+    /* Removed: Causes traps on Windows XP
+       res = addItem(myHwnd, -2, "Out Of Range Item");
+       ok(res == -1, "Adding out of range worked unexpectedly (%ld)\n", res);
+     */
+
+    /* Get an item completely out of range */ 
+    res = getItem(myHwnd, 99, &cbexItem); 
+    ok(res == 0, "Getting item using out of range index worked unexpectedly (%ld, %s)\n", res, cbexItem.pszText);
+    res = getItem(myHwnd, 4, &cbexItem); 
+    ok(res == 0, "Getting item using out of range index worked unexpectedly (%ld, %s)\n", res, cbexItem.pszText);
+    res = getItem(myHwnd, -2, &cbexItem); 
+    ok(res == 0, "Getting item using out of range index worked unexpectedly (%ld, %s)\n", res, cbexItem.pszText);
+
+    /* Get an item in range */ 
+    res = getItem(myHwnd, 0, &cbexItem); 
+    ok(res != 0, "Getting item using valid index failed unexpectedly (%ld)\n", res);
+    ok(strcmp(FIRST_ITEM, cbexItem.pszText) == 0, "Getting item returned wrong string (%s)\n", cbexItem.pszText);
+
+    res = getItem(myHwnd, 1, &cbexItem); 
+    ok(res != 0, "Getting item using valid index failed unexpectedly (%ld)\n", res);
+    ok(strcmp(MIDDLE_ITEM, cbexItem.pszText) == 0, "Getting item returned wrong string (%s)\n", cbexItem.pszText);
+
+    res = getItem(myHwnd, 2, &cbexItem); 
+    ok(res != 0, "Getting item using valid index failed unexpectedly (%ld)\n", res);
+    ok(strcmp(SECOND_ITEM, cbexItem.pszText) == 0, "Getting item returned wrong string (%s)\n", cbexItem.pszText);
+
+    res = getItem(myHwnd, 3, &cbexItem); 
+    ok(res != 0, "Getting item using valid index failed unexpectedly (%ld)\n", res);
+    ok(strcmp(THIRD_ITEM, cbexItem.pszText) == 0, "Getting item returned wrong string (%s)\n", cbexItem.pszText);
+
+    /* Set an item completely out of range */ 
+    res = setItem(myHwnd, 99, REPLACEMENT_ITEM); 
+    ok(res == 0, "Setting item using out of range index worked unexpectedly (%ld)\n", res);
+    res = setItem(myHwnd, 4, REPLACEMENT_ITEM); 
+    ok(res == 0, "Setting item using out of range index worked unexpectedly (%ld)\n", res);
+    res = setItem(myHwnd, -2, REPLACEMENT_ITEM); 
+    ok(res == 0, "Setting item using out of range index worked unexpectedly (%ld)\n", res);
+
+    /* Set an item in range */ 
+    res = setItem(myHwnd, 0, REPLACEMENT_ITEM);
+    ok(res != 0, "Setting first item failed (%ld)\n", res);
+    res = setItem(myHwnd, 3, REPLACEMENT_ITEM);
+    ok(res != 0, "Setting last item failed (%ld)\n", res);
+
+    /* Remove items completely out of range (4 items in control at this point) */
+    res = delItem(myHwnd, -1);
+    ok(res == CB_ERR, "Deleting using out of range index worked unexpectedly (%ld)\n", res);
+    res = delItem(myHwnd, 4);
+    ok(res == CB_ERR, "Deleting using out of range index worked unexpectedly (%ld)\n", res);
+
+    /* Remove items in range (4 items in control at this point) */
+    res = delItem(myHwnd, 3);
+    ok(res == 3, "Deleting using out of range index failed (%ld)\n", res);
+    res = delItem(myHwnd, 0);
+    ok(res == 2, "Deleting using out of range index failed (%ld)\n", res);
+    res = delItem(myHwnd, 0);
+    ok(res == 1, "Deleting using out of range index failed (%ld)\n", res);
+    res = delItem(myHwnd, 0);
+    ok(res == 0, "Deleting using out of range index failed (%ld)\n", res);
+
+    /* Remove from an empty box */
+    res = delItem(myHwnd, 0);
+    ok(res == CB_ERR, "Deleting using out of range index worked unexpectedly (%ld)\n", res);
+
+
+    /* Cleanup */
+    free(textBuffer);
+
+}
+
+LRESULT CALLBACK ComboExTestWndProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam)
+{
+    switch(msg) {
+    
+    case WM_DESTROY:
+        PostQuitMessage(0);
+        break;
+  
+    default:
+        return DefWindowProcA(hWnd, msg, wParam, lParam);
+    }
+    
+    return 0L;
+}
+
+static void init(void) {
+    WNDCLASSA wc;
+    INITCOMMONCONTROLSEX icex;
+
+    icex.dwSize = sizeof(INITCOMMONCONTROLSEX);
+    icex.dwICC   = ICC_USEREX_CLASSES;
+    InitCommonControlsEx(&icex);
+
+    wc.style = CS_HREDRAW | CS_VREDRAW;
+    wc.cbClsExtra = 0;
+    wc.cbWndExtra = 0;
+    wc.hInstance = GetModuleHandleA(NULL);
+    wc.hIcon = NULL;
+    wc.hCursor = LoadCursorA(NULL, MAKEINTRESOURCEA(IDC_ARROW));
+    wc.hbrBackground = GetSysColorBrush(COLOR_WINDOW);
+    wc.lpszMenuName = NULL;
+    wc.lpszClassName = ComboExTestClass;
+    wc.lpfnWndProc = ComboExTestWndProc;
+    RegisterClassA(&wc);
+
+    hComboExParentWnd = CreateWindowExA(0, ComboExTestClass, "ComboEx test", WS_OVERLAPPEDWINDOW, 
+      CW_USEDEFAULT, CW_USEDEFAULT, 680, 260, NULL, NULL, GetModuleHandleA(NULL), 0);
+    assert(hComboExParentWnd != NULL);
+
+    hMainHinst = GetModuleHandleA(NULL);
+
+}
+
+static void cleanup(void)
+{
+    MSG msg;
+    
+    PostMessageA(hComboExParentWnd, WM_CLOSE, 0, 0);
+    while (GetMessageA(&msg,0,0,0)) {
+        TranslateMessage(&msg);
+        DispatchMessageA(&msg);
+    }
+    
+    UnregisterClassA(ComboExTestClass, GetModuleHandleA(NULL));
+}
+
+START_TEST(comboex)
+{
+    init();
+
+    test_comboboxex();
+
+    cleanup();
+}