Index: generic/tkBind.c ================================================================== --- generic/tkBind.c +++ generic/tkBind.c @@ -141,11 +141,11 @@ /* We need this array for bookkeeping the last matching modifier mask per pattern. */ TK_ARRAY_DEFINE(PSModMaskArr, unsigned); typedef struct PSEntry { - TK_DLIST_LINKS(PSEntry); /* Makes this struct a double linked list; must be first entry. */ + TK_DLIST_LINKS(PSEntry); /* Makes this struct a doubly linked list; must be first entry. */ Window window; /* Window of last match. */ struct PatSeq* psPtr; /* Pointer to pattern sequence. */ PSModMaskArr *lastModMaskArr; /* Last matching modifier mask per pattern (except last pattern). * Only needed if pattern sequence is not single (more than one @@ -186,11 +186,11 @@ * application with separate event bindings for each (for example, each canvas * widget has a separate binding table for associating events with the items * in the canvas). */ -/* defining the whole Promotion_* stuff (array of PSList entries) */ +/* Defining the whole PromArr_* stuff (array of PSList entries) */ TK_ARRAY_DEFINE(PromArr, PSList); typedef struct Tk_BindingTable_ { Event eventInfo[TK_LASTEVENT]; /* Containing the most recent event for every event type. */ @@ -417,11 +417,11 @@ * Flags for ModInfo structures: * * DOUBLE - Non-zero means duplicate this event, e.g. for double-clicks. * TRIPLE - Non-zero means triplicate this event, e.g. for triple-clicks. * QUADRUPLE - Non-zero means quadruple this event, e.g. for 4-fold-clicks. - * MULT_CLICKS - Combination of all of above. + * MULT_CLICKS - Combination of all the above. */ #define DOUBLE (1<<0) #define TRIPLE (1<<1) #define QUADRUPLE (1<<2) @@ -554,11 +554,11 @@ #define RESIZEREQ (1<<22) #define CIRCREQ (1<<23) /* * These structs agree with xkey for the fields type, serial, send_event, display, - * window, root, subwindow, time, x, y, x_root, and y_root. So when accessing + * window, root, subwindow, time, x, y, x_root, and y_root. So when accessing * these fields we may pretend that we are using a struct xkey. */ #define HAS_XKEY_HEAD (KEY|BUTTON|MOTION|VIRTUAL|CROSSING|WHEEL) @@ -752,12 +752,12 @@ static int TestNearbyTime(int lhs, int rhs) { return Abs(lhs - rhs) <= NEARBY_MS; } static int TestNearbyCoords(int lhs, int rhs) { return Abs(lhs - rhs) <= NEARBY_PIXELS; } static int IsSubsetOf( - unsigned lhsMask, /* this is a subset */ - unsigned rhsMask) /* of this bit field? */ + unsigned lhsMask, /* Is this a subset... */ + unsigned rhsMask) /* ...of this bit field? */ { return (lhsMask & rhsMask) == lhsMask; } static const char* @@ -856,12 +856,12 @@ return eventType == ButtonPress || eventType == ButtonRelease; } static int MatchEventNearby( - const XEvent *lhs, /* previous button event */ - const XEvent *rhs) /* current button event */ + const XEvent *lhs, /* Previous button event */ + const XEvent *rhs) /* Current button event */ { assert(lhs); assert(rhs); assert(IsButtonEventType(lhs->type)); assert(lhs->type == rhs->type); @@ -873,12 +873,12 @@ && TestNearbyCoords(rhs->xbutton.y_root, lhs->xbutton.y_root); } static int MatchEventRepeat( - const XKeyEvent *lhs, /* previous key event */ - const XKeyEvent *rhs) /* current key event */ + const XKeyEvent *lhs, /* Previous key event */ + const XKeyEvent *rhs) /* Current key event */ { assert(lhs); assert(rhs); assert(IsKeyEventType(lhs->type)); assert(lhs->type == rhs->type); @@ -1099,11 +1099,11 @@ Tcl_HashEntry *hPtr; assert(lookupTables); assert(eventPtr); - /* otherwise on some systems the key contains uninitialized bytes */ + /* Otherwise on some systems the key contains uninitialized bytes. */ memset(&key, 0, sizeof(PatternTableKey)); if (onlyConsiderDetailedEvents) { switch (eventPtr->xev.type) { case ButtonPress: /* fallthru */ @@ -1230,11 +1230,11 @@ * *--------------------------------------------------------------------------- */ /* - * Windoze compiler does not allow the definition of these static variables inside a function, + * Windows compiler does not allow the definition of these static variables inside a function, * otherwise this should belong to function TkBindInit(). */ TCL_DECLARE_MUTEX(bindMutex); static int initialized = 0; @@ -1244,41 +1244,41 @@ { BindInfo *bindInfoPtr; assert(mainPtr); - /* otherwise virtual events can't be supported */ + /* Otherwise virtual events can't be supported. */ assert(sizeof(XEvent) >= sizeof(XVirtualEvent)); - /* type of TkPattern.info is well defined? */ + /* Is type of TkPattern.info well defined? */ assert(sizeof(Info) >= sizeof(KeySym)); assert(sizeof(Info) >= sizeof(unsigned)); - /* ensure that our matching algorithm is working (when testing detail) */ + /* Ensure that our matching algorithm is working (when testing detail). */ assert(sizeof(Detail) == sizeof(Tk_Uid)); - /* test expected indices of Button1..Button5, otherwise our button handling is not working */ + /* Test expected indices of Button1..Button5, otherwise our button handling is not working. */ assert(Button1 == 1 && Button2 == 2 && Button3 == 3 && Button4 == 4 && Button5 == 5); assert(Button2Mask == (Button1Mask << 1)); assert(Button3Mask == (Button1Mask << 2)); assert(Button4Mask == (Button1Mask << 3)); assert(Button5Mask == (Button1Mask << 4)); - /* test expected values of button motion masks, otherwise our button handling is not working */ + /* Test expected values of button motion masks, otherwise our button handling is not working. */ assert(Button1MotionMask == Button1Mask); assert(Button2MotionMask == Button2Mask); assert(Button3MotionMask == Button3Mask); assert(Button4MotionMask == Button4Mask); assert(Button5MotionMask == Button5Mask); - /* because we expect zero if keySym is empty */ + /* Because we expect zero if keySym is empty. */ assert(NoSymbol == 0L); - /* this must be a union, not a struct, otherwise comparison with NULL will not work */ + /* This must be a union, not a struct, otherwise comparison with NULL will not work. */ assert(Tk_Offset(Detail, name) == Tk_Offset(Detail, info)); - /* we use some constraints about X*Event */ + /* We use some constraints about X*Event. */ assert(Tk_Offset(XButtonEvent, time) == Tk_Offset(XMotionEvent, time)); assert(Tk_Offset(XButtonEvent, x_root) == Tk_Offset(XMotionEvent, x_root)); assert(Tk_Offset(XButtonEvent, y_root) == Tk_Offset(XMotionEvent, y_root)); assert(Tk_Offset(XCreateWindowEvent, border_width) == Tk_Offset(XConfigureEvent, border_width)); assert(Tk_Offset(XCreateWindowEvent, width) == Tk_Offset(XConfigureEvent, width)); @@ -2240,28 +2240,28 @@ if (eventPtr->xkey.time) { bindInfoPtr->lastCurrentTime = CurrentTimeInMilliSecs(); bindInfoPtr->lastEventTime = eventPtr->xkey.time; } - /* modifier keys should not influence button events */ + /* Modifier keys should not influence button events. */ for (i = 0; i < (unsigned) dispPtr->numModKeyCodes; ++i) { if (dispPtr->modKeyCodes[i] == eventPtr->xkey.keycode) { reset = 0; } } if (reset) { - /* reset repetition count for button events */ + /* Reset repetition count for button events. */ bindPtr->eventInfo[ButtonPress].countAny = 0; bindPtr->eventInfo[ButtonPress].countDetailed = 0; bindPtr->eventInfo[ButtonRelease].countAny = 0; bindPtr->eventInfo[ButtonRelease].countDetailed = 0; } break; } case ButtonPress: case ButtonRelease: - /* reset repetition count for key events */ + /* Reset repetition count for key events. */ bindPtr->eventInfo[KeyPress].countAny = 0; bindPtr->eventInfo[KeyPress].countDetailed = 0; bindPtr->eventInfo[KeyRelease].countAny = 0; bindPtr->eventInfo[KeyRelease].countDetailed = 0; /* fallthru */ @@ -2361,11 +2361,11 @@ scriptCount = 0; arraySize = 0; Tcl_DStringInit(&scripts); if ((size_t) numObjects > SIZE_OF_ARRAY(matchPtrBuf)) { - /* it's unrealistic that the buffer size is too small, but who knows? */ + /* It's unrealistic that the buffer size is too small, but who knows? */ matchPtrArr = (PatSeq **)ckalloc(numObjects*sizeof(matchPtrArr[0])); } memset(matchPtrArr, 0, numObjects*sizeof(matchPtrArr[0])); if (!PromArr_IsEmpty(bindPtr->promArr)) { @@ -2383,15 +2383,15 @@ for (i = PromArr_Size(bindPtr->promArr); i > 0; --i, --psl[0], --psl[1]) { psPtr[0] = MatchPatterns(dispPtr, bindPtr, psl[0], psl[1], i, curEvent, objArr[k], NULL); if (IsBetterMatch(matchPtrArr[k], psPtr[0])) { - /* we will process it later, because we still may find a pattern with better match */ + /* We will process it later, because we still may find a pattern with better match. */ matchPtrArr[k] = psPtr[0]; } if (!PSList_IsEmpty(psl[1])) { - /* we have promoted sequences, adjust array size */ + /* We have promoted sequences, adjust array size. */ arraySize = Max(i + 1, arraySize); } } } } @@ -2412,11 +2412,11 @@ psPtr[0] = MatchPatterns(dispPtr, bindPtr, psl[0], psSuccList, 0, curEvent, objArr[k], NULL); psPtr[1] = MatchPatterns(dispPtr, bindPtr, psl[1], psSuccList, 0, curEvent, objArr[k], NULL); if (!PSList_IsEmpty(psSuccList)) { - /* we have promoted sequences, adjust array size */ + /* We have promoted sequences, adjust array size. */ arraySize = Max(1u, arraySize); } bestPtr = psPtr[0] ? psPtr[0] : psPtr[1]; @@ -2465,11 +2465,11 @@ } } if (matchPtrArr[k]) { ExpandPercents(winPtr, matchPtrArr[k]->script, curEvent, scriptCount++, &scripts); - /* nul is added to the scripts string to separate the various scripts */ + /* Null is added to the scripts string to separate the various scripts. */ Tcl_DStringAppend(&scripts, "", 1); } } PromArr_SetSize(bindPtr->promArr, arraySize); @@ -2530,11 +2530,11 @@ } } } if (!PSList_IsEmpty(psList)) { - /* we still have promoted sequences, adjust array size */ + /* We still have promoted sequences, adjust array size. */ newArraySize = Max(i + 1, newArraySize); } } PromArr_SetSize(bindPtr->promArr, newArraySize); @@ -2542,11 +2542,11 @@ if (matchPtrArr != matchPtrBuf) { ckfree(matchPtrArr); } if (Tcl_DStringLength(&scripts) == 0) { - return; /* nothing to do */ + return; /* Nothing to do. */ } /* * Now go back through and evaluate the binding for each object, in order, * dealing with "break" and "continue" exceptions appropriately. @@ -2673,16 +2673,16 @@ const TkPattern *physPatPtr = (*physPtrPtr)->pats; const TkPattern *virtPatPtr = psPtr->pats; if (physPatPtr->info || !virtPatPtr->info) { if (IsSubsetOf(virtPatPtr->modMask, physPatPtr->modMask)) { - return 0; /* we cannot surpass this match */ + return 0; /* We cannot surpass this match. */ } } } - /* otherwise on some systems the key contains uninitialized bytes */ + /* Otherwise on some systems the key contains uninitialized bytes. */ memset(&key, 0, sizeof(key)); key.object = object; key.type = VirtualEvent; owners = psPtr->ptr.owners; @@ -2704,11 +2704,11 @@ /* helper function */ static int Compare( const PatSeq *fstMatchPtr, - const PatSeq *sndMatchPtr) /* most recent match */ + const PatSeq *sndMatchPtr) /* Most recent match. */ { int diff; if (!fstMatchPtr) { return +1; } assert(sndMatchPtr); @@ -2759,10 +2759,24 @@ if (IsSubsetOf(fstModMask, sndModMask)) { ++sndCount; } if (IsSubsetOf(sndModMask, fstModMask)) { ++fstCount; } return fstCount - sndCount; } + +/* helper function */ +static int +IsPSInPSList( + const PatSeq *psPtr, /* Is this pattern sequence... */ + const PSList *psList) /* ...an element of this list of patterns sequence? */ +{ + PSEntry *psEntry; + + TK_DLIST_FOREACH(psEntry, psList) { + if (psEntry->psPtr == psPtr) { return 1; } + } + return 0; +} static PatSeq * MatchPatterns( TkDisplay *dispPtr, /* Display from which the event came. */ Tk_BindingTable bindPtr, /* Table in which to look for bindings. */ @@ -2829,11 +2843,11 @@ if (psPtr->object ? psPtr->object == object : VirtPatIsBound(bindPtr, psPtr, object, physPtrPtr)) { TkPattern *patPtr = psPtr->pats + patIndex; - /* ignore modifier key events, and KeyRelease events if the current event + /* Ignore modifier key events, and KeyRelease events if the current event * is of a different type (e.g. a Button event) */ psEntry->keepIt = isModKeyOnly || \ ((patPtr->eventType != (unsigned) curEvent->xev.type) && curEvent->xev.type == KeyRelease); @@ -2848,12 +2862,12 @@ * be the better place. */ unsigned modMask = ResolveModifiers(dispPtr, patPtr->modMask); unsigned curModMask = ResolveModifiers(dispPtr, bindPtr->curModMask); - psEntry->expired = 1; /* remove it from promotion list */ - psEntry->keepIt = 0; /* don't keep matching patterns */ + psEntry->expired = 1; /* Remove it from promotion list. */ + psEntry->keepIt = 0; /* Don't keep matching patterns. */ if (IsSubsetOf(modMask, curModMask)) { unsigned count = patPtr->info ? curEvent->countDetailed : curEvent->countAny; if (patIndex < PSModMaskArr_Size(psEntry->lastModMaskArr)) { @@ -2865,11 +2879,11 @@ */ if (psPtr->numPats == patIndex + 1) { if (patPtr->count <= count) { /* - * This is also a final pattern. + * This is also a final pattern (i.e. the pattern sequence is complete). * We always prefer the pattern with better match. * If completely equal than prefer most recently defined pattern. */ int cmp = Compare(bestPtr, psPtr); @@ -2887,35 +2901,45 @@ bestPhysPtr = *physPtrPtr; } } } else { DEBUG(psEntry->expired = 0;) - psEntry->keepIt = 1; /* don't remove it from promotion list */ + psEntry->keepIt = 1; /* Don't remove it from promotion list. */ } } else if (psSuccList) { /* - * Not a final pattern, but matching, so promote it to next level. + * Not a final pattern, but matching (i.e. successive patterns match the pattern sequence so far), + * so promote the pattern sequence to next level if not already promoted in the success list. * But do not promote if count of current pattern is not yet reached. */ - if (patPtr->count == psEntry->count) { - PSEntry *psNewEntry; - - assert(!patPtr->name); - psNewEntry = MakeListEntry( - &bindPtr->lookupTables.entryPool, psPtr, psPtr->modMaskUsed); - if (!PSModMaskArr_IsEmpty(psNewEntry->lastModMaskArr)) { - PSModMaskArr_Set(psNewEntry->lastModMaskArr, patIndex, &modMask); - } - assert(psNewEntry->keepIt); - assert(psNewEntry->count == 1u); - PSList_Append(psSuccList, psNewEntry); - psNewEntry->window = window; /* bind to current window */ + if (!IsPSInPSList(psPtr, psSuccList)) { + if (patPtr->count == psEntry->count) { + PSEntry *psNewEntry; + + assert(!patPtr->name); + psNewEntry = MakeListEntry( + &bindPtr->lookupTables.entryPool, psPtr, psPtr->modMaskUsed); + if (!PSModMaskArr_IsEmpty(psNewEntry->lastModMaskArr)) { + PSModMaskArr_Set(psNewEntry->lastModMaskArr, patIndex, &modMask); + } + assert(psNewEntry->keepIt); + assert(psNewEntry->count == 1u); + PSList_Append(psSuccList, psNewEntry); + psNewEntry->window = window; /* Bind to current window. */ + } else { + assert(psEntry->count < patPtr->count); + DEBUG(psEntry->expired = 0;) + psEntry->count += 1; + psEntry->keepIt = 1; /* Don't remove it from promotion list. */ + } } else { - assert(psEntry->count < patPtr->count); + /* + * Pattern sequence is already present in the success list. + */ + DEBUG(psEntry->expired = 0;) - psEntry->count += 1; - psEntry->keepIt = 1; /* don't remove it from promotion list */ + psEntry->keepIt = 1; /* Don't remove it from promotion list. */ } } } } } @@ -3547,11 +3571,11 @@ if (!(virtUid = GetVirtualEventUid(interp, virtString))) { return 0; } /* - * Find/create physical event + * Find/create physical event. */ if (!(psPtr = FindSequence(interp, &vetPtr->lookupTables, NULL, eventString, 1, 0, NULL))) { return 0; } @@ -3655,11 +3679,11 @@ if (!eventPSPtr || psPtr == eventPSPtr) { VirtOwners *owners = psPtr->ptr.owners; int iVirt = VirtOwners_Find(owners, vhPtr); - assert(iVirt != -1); /* otherwise we couldn't find owner, and this should not happen */ + assert(iVirt != -1); /* Otherwise we couldn't find owner, and this should not happen. */ /* * Remove association between this physical event and the given * virtual event that it triggers. */ @@ -4835,11 +4859,11 @@ assert(eventStringPtr); assert(patPtr); assert(eventMaskPtr); p = *eventStringPtr; - memset(patPtr, 0, sizeof(TkPattern)); /* otherwise memcmp doesn't work */ + memset(patPtr, 0, sizeof(TkPattern)); /* Otherwise memcmp doesn't work. */ /* * Handle simple ASCII characters. */ @@ -4966,11 +4990,11 @@ unsigned button = GetButtonNumber(field); if ((eventFlags & BUTTON) || (button && eventFlags == 0) || (SUPPORT_ADDITIONAL_MOTION_SYNTAX && (eventFlags & MOTION) && button == 0)) { - /* This must be a button (or bad motion) event */ + /* This must be a button (or bad motion) event. */ if (button == 0) { return FinalizeParseEventDescription( interp, patPtr, 0, Tcl_ObjPrintf("bad button number \"%s\"", field), "BUTTON"); Index: tests/bind.test ================================================================== --- tests/bind.test +++ tests/bind.test @@ -6984,10 +6984,31 @@ && $y1==[winfo rooty .top.l]+10} } -cleanup { destroy .top unset x1 y1 x2 y2 } -result 1 + +test bind-37.1 {Promotion tables do not contain duplicate sequences, bug [43573999ca]} -body { + proc A {} { + bind .c {} + set myv(a) 1 + set b [array get myv] + bind .c "puts Trigger" + } + pack [canvas .c] + bind .c "A" + A + update + event generate .c + event generate .c + event generate .c + event generate .c + event generate .c + event generate .c +} -cleanup { + destroy .c +} -returnCodes ok -result {} ; # shall not crash (assertion failed) # cleanup cleanupTests return