Menu

#3406 Tcl_GetIndexFromObj doesn't like zero length strings

obsolete: 8.5a4
closed-fixed
7
2006-04-05
2006-04-04
No

See Tk bug 1463233 for more details:

https://sourceforge.net/tracker/?func=detail&atid=112997&aid=1463233&group_id=12997

Basically, Tk's text widget tag option configuration
allows (naturally) the empty string as a valid
configuration value (to indicate that the tag does not
override the text widget for that item). This works
for all other configuration types, but not for indices
from string lists. The root cause of the problem is
that Tcl_GetIndexFromObj throws an error in this case,
even though there is an exact match available (and
nowhere in the documentation is it stated that an exact
match will not be found for the special case of an
empty string).

An apparent fix would be:

/*
* The key should not be empty, otherwise it's not a
match.
*/

if (key[0] == '\0') {
+
+ /*
+ * Check for special case of "" in string
table.
+ */
+ for (entryPtr = tablePtr, i = 0; *entryPtr !=
NULL;
+ entryPtr = NEXT_ENTRY(entryPtr, offset), i++) {
+ if (**entryPtr == '\0') {
+ index = i;
+ goto done;
+ }
+ }
+
goto error;
}

/*
* Scan the table looking for one of:

Note that this does NOT give the empty string any
special treatment - it simply equalises the treatment
with what happens with any other prefix string. If
there is an exact match to any prefix string it is
accepted, even if there are longer strings which could
also match.

Vince.

Discussion

  • Don Porter

    Don Porter - 2006-04-04

    Logged In: YES
    user_id=80530

    I get it now thanks.
    This is a bug that needs
    a fix.

    Isn't it a simpler fix
    to just delete lines
    196-203 of tclIndexObj.c ?

     
  • Nobody/Anonymous

    Logged In: NO

    Indeed, you're right. The better fix is to remove those
    lines (although would want to check if that causes any tcl
    test suite problems).

    Vince.

     
  • Donal K. Fellows

    • assigned_to: dgp --> dkf
    • status: open --> open-fixed
     
  • Donal K. Fellows

    Logged In: YES
    user_id=79902

    Fixed in 8.5 (empty string is exact-matched only; never a
    prefix). Also added tests.

     
  • Donal K. Fellows

    • status: open-fixed --> closed-fixed
     
  • Donal K. Fellows

    Logged In: YES
    user_id=79902

    Fixed on 8.4 branch too

     
  • Don Porter

    Don Porter - 2006-04-05

    Logged In: YES
    user_id=80530

    note that this changes some
    error messages causing some
    other test suite failures.

    Also, this patch fails to
    accept the empty string
    as a unique prefix for
    the option in a single option
    table. That doesn't make
    sense to me.

     
  • Don Porter

    Don Porter - 2006-04-05

    Logged In: YES
    user_id=80530

    revisions committed.

     
  • Don Porter

    Don Porter - 2006-04-06

    Logged In: YES
    user_id=80530

    revisions reverted.
    dkf called this one right,
    thinking more carefully
    about the expectations
    of existing extensions.

    I dislike the inconsistency,
    but I guess I dislike breaking
    things more. :P:P:P

    Committing the change back...