More "Death to defensive programming"...
ChangeLog
Assert on inconsistent range list states
Various code cleanups, few potential bugs fixed.
--- dlls/comctl32/listview.c.V4 Sun Oct 20 11:20:11 2002
+++ dlls/comctl32/listview.c Sun Oct 20 12:29:10 2002
@@ -949,11 +949,6 @@
if (IntersectRect(&rcTemp, &rcItem, &frame))
ranges_additem(i->ranges, nItem);
}
- if (TRACE_ON(listview))
- {
- TRACE(" icon items:\n");
- ranges_dump(i->ranges);
- }
return TRUE;
}
else if (uView == LVS_REPORT)
@@ -2222,27 +2217,28 @@
}
#if DEBUG_RANGES
-#define ranges_check ranges_assert
+#define ranges_check(ranges, desc) ranges_assert(ranges, desc, __FUNCTION__, __LINE__)
#else
#define ranges_check(ranges, desc) do { } while(0)
#endif
-static void ranges_assert(RANGES ranges, LPCSTR desc)
+static void ranges_assert(RANGES ranges, LPCSTR desc, const char *func, int line)
{
INT i;
RANGE *prev, *curr;
+ TRACE("*** Checking %s:%d:%s ***\n", func, line, desc);
assert (ranges);
assert (ranges->hdpa->nItemCount >= 0);
- if (ranges->hdpa->nItemCount == 0) return;
- TRACE("*** Checking %s ***\n", desc);
ranges_dump(ranges);
- assert ((prev = (RANGE *)DPA_GetPtr(ranges->hdpa, 0))->lower >= 0);
- /* assert (((RANGE *)DPA_GetPtr(ranges->hdpa, ranges->hdpa->nItemCount - 1))->upper <= nUpper); */
+ prev = (RANGE *)DPA_GetPtr(ranges->hdpa, 0);
+ if (ranges->hdpa->nItemCount > 0)
+ assert (prev->lower >= 0 && prev->lower < prev->upper);
for (i = 1; i < ranges->hdpa->nItemCount; i++)
{
curr = (RANGE *)DPA_GetPtr(ranges->hdpa, i);
assert (prev->upper <= curr->lower);
+ assert (curr->lower < curr->upper);
prev = curr;
}
TRACE("--- Done checking---\n");
@@ -2260,8 +2256,7 @@
static void ranges_destroy(RANGES ranges)
{
- if (!ranges) return;
- if (ranges->hdpa)
+ if (ranges && ranges->hdpa)
{
INT i;
@@ -2278,30 +2273,27 @@
RANGES clone;
INT i;
- if (!ranges) return NULL;
- clone = ranges_create(ranges->hdpa->nItemCount);
- if (!clone) return NULL;
-
+ if (!(clone = ranges_create(ranges->hdpa->nItemCount))) goto fail;
+
for (i = 0; i < ranges->hdpa->nItemCount; i++)
{
RANGE *newrng = (RANGE *)COMCTL32_Alloc(sizeof(RANGE));
- if (!newrng)
- {
- ranges_destroy(clone);
- return NULL;
- }
+ if (!newrng) goto fail;
*newrng = *((RANGE*)DPA_GetPtr(ranges->hdpa, i));
DPA_InsertPtr(clone->hdpa, i, newrng);
}
return clone;
+
+fail:
+ TRACE ("clone failed\n");
+ if (clone) ranges_destroy(clone);
+ return NULL;
}
static RANGES ranges_diff(RANGES ranges, RANGES sub)
{
INT i;
- if (!ranges || !sub) return ranges;
-
for (i = 0; i < sub->hdpa->nItemCount; i++)
ranges_del(ranges, *((RANGE *)DPA_GetPtr(sub->hdpa, i)));
@@ -2312,7 +2304,6 @@
{
INT i;
- if (!ranges) return;
for (i = 0; i < ranges->hdpa->nItemCount; i++)
TRACE(" %s\n", debugrange(DPA_GetPtr(ranges->hdpa, i)));
}
@@ -2322,8 +2313,6 @@
RANGE srchrng = { nItem, nItem + 1 };
TRACE("(nItem=%d)\n", nItem);
- if (!ranges) return FALSE;
- if (TRACE_ON(listview)) ranges_dump(ranges);
ranges_check(ranges, "before contain");
return DPA_Search(ranges->hdpa, &srchrng, 0, ranges_cmp, 0, DPAS_SORTED) != -1;
}
@@ -2332,7 +2321,6 @@
{
INT i, count = 0;
- if (!ranges) return 0;
for (i = 0; i < ranges->hdpa->nItemCount; i++)
{
RANGE *sel = DPA_GetPtr(ranges->hdpa, i);
@@ -2347,11 +2335,10 @@
RANGE srchrng = { nItem, nItem + 1 }, *chkrng;
INT index;
- if (!ranges) return FALSE;
index = DPA_Search(ranges->hdpa, &srchrng, 0, ranges_cmp, 0, DPAS_SORTED | DPAS_INSERTAFTER);
if (index == -1) return TRUE;
- for (;index < ranges->hdpa->nItemCount; index++)
+ for (; index < ranges->hdpa->nItemCount; index++)
{
chkrng = DPA_GetPtr(ranges->hdpa, index);
if (chkrng->lower >= nItem)
@@ -2369,7 +2356,6 @@
TRACE("(%s)\n", debugrange(&range));
ranges_check(ranges, "before add");
- if (!ranges) goto fail;
/* try find overlapping regions first */
srchrgn.lower = range.lower - 1;
@@ -2393,7 +2379,11 @@
if (index == -1) index = 0;
/* and get it over with */
- DPA_InsertPtr(ranges->hdpa, index, newrgn);
+ if (DPA_InsertPtr(ranges->hdpa, index, newrgn) == -1)
+ {
+ COMCTL32_Free(newrgn);
+ goto fail;
+ }
}
else
{
@@ -2401,7 +2391,6 @@
INT fromindex, mergeindex;
chkrgn = DPA_GetPtr(ranges->hdpa, index);
- if (!chkrgn) goto fail;
TRACE("Merge with %s @%d\n", debugrange(chkrgn), index);
chkrgn->lower = min(range.lower, chkrgn->lower);
@@ -2427,8 +2416,6 @@
TRACE("Merge with index %i\n", mergeindex);
mrgrgn = DPA_GetPtr(ranges->hdpa, mergeindex);
- if (!mrgrgn) goto fail;
-
chkrgn->lower = min(chkrgn->lower, mrgrgn->lower);
chkrgn->upper = max(chkrgn->upper, mrgrgn->upper);
COMCTL32_Free(mrgrgn);
@@ -2438,7 +2425,6 @@
}
ranges_check(ranges, "after add");
- if (TRACE_ON(listview)) ranges_dump(ranges);
return TRUE;
fail:
@@ -2453,7 +2439,6 @@
TRACE("(%s)\n", debugrange(&range));
ranges_check(ranges, "before del");
- if (!ranges) goto fail;
/* we don't use DPAS_SORTED here, since we need *
* to find the first overlapping range */
@@ -2461,7 +2446,6 @@
while(index != -1)
{
chkrgn = DPA_GetPtr(ranges->hdpa, index);
- if (!chkrgn) goto fail;
TRACE("Matches range %s @%d\n", debugrange(chkrgn), index);
@@ -2500,7 +2484,11 @@
newrgn->lower = chkrgn->lower;
newrgn->upper = range.lower;
chkrgn->lower = range.upper;
- DPA_InsertPtr(ranges->hdpa, index, newrgn);
+ if (DPA_InsertPtr(ranges->hdpa, index, newrgn) == -1)
+ {
+ COMCTL32_Free(newrgn);
+ goto fail;
+ }
chkrgn = &tmprgn;
break;
}
@@ -2532,15 +2520,16 @@
{
LVITEMW lvItem;
ITERATOR i;
+ RANGES clone;
TRACE("()\n");
- if (TRACE_ON(listview)) ranges_dump(toSkip);
lvItem.state = 0;
lvItem.stateMask = LVIS_SELECTED;
/* need to clone the DPA because callbacks can change it */
- iterator_ranges(&i, ranges_diff(ranges_clone(infoPtr->selectionRanges), toSkip));
+ if (!(clone = ranges_clone(infoPtr->selectionRanges))) return FALSE;
+ iterator_ranges(&i, ranges_diff(clone, toSkip));
while(iterator_next(&i))
LISTVIEW_SetItemState(infoPtr, i.nItem, &lvItem);
/* note that the iterator destructor will free the cloned range */
@@ -2551,9 +2540,9 @@
static inline BOOL LISTVIEW_DeselectAllSkipItem(LISTVIEW_INFO *infoPtr, INT nItem)
{
- RANGES toSkip = ranges_create(1);
+ RANGES toSkip;
- if (!toSkip) return FALSE;
+ if (!(toSkip = ranges_create(1))) return FALSE;
if (nItem != -1) ranges_additem(toSkip, nItem);
LISTVIEW_DeselectAllSkipItems(infoPtr, toSkip);
ranges_destroy(toSkip);
@@ -6878,6 +6867,9 @@
/* set header font */
SendMessageW(infoPtr->hwndHeader, WM_SETFONT, (WPARAM)infoPtr->hFont, (LPARAM)TRUE);
+ /* allocate memory for the selection ranges */
+ if (!(infoPtr->selectionRanges = ranges_create(10))) return -1;
+
infoPtr->hdpaColumns = DPA_Create(10);
if (uView == LVS_ICON)
@@ -6919,9 +6911,6 @@
infoPtr->hdpaPosX = DPA_Create(10);
infoPtr->hdpaPosY = DPA_Create(10);
- /* allocate memory for the selection ranges */
- infoPtr->selectionRanges = ranges_create(10);
-
/* initialize size of items */
infoPtr->nItemWidth = LISTVIEW_CalculateMaxWidth(infoPtr);
infoPtr->nItemHeight = LISTVIEW_CalculateMaxHeight(infoPtr);
