Fix for [Bug 1288128]

2 files changed, 140 insertions, 86 deletions

diff --git a/ChangeLog b/ChangeLog
index 06e167e..4feedff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2005-09-15  Donal K. Fellows  <donal.k.fellows@man.ac.uk>
+
+	* generic/tkOldConfig.c (GetCachedSpecs): Split out the code to
+	manipulate the cached writable specs so that it can be reused from
+	all the public Tk_Configure* functions.
+	(Tk_ConfigureInfo, Tk_ConfigureWidget, Tk_ConfigureValue): Use the
+	factored out code everywhere, so we always manipulate the cache
+	correctly. [Bug 1288128]
+
 2005-09-11  Daniel Steffen  <das@users.sourceforge.net>
 
 	* macosx/tkMacOSXMouseEvent.c (TkMacOSXProcessMouseEvent): check if
diff --git a/generic/tkOldConfig.c b/generic/tkOldConfig.c
index e956c2b..1f37725 100644
--- a/generic/tkOldConfig.c
+++ b/generic/tkOldConfig.c
@@ -11,7 +11,7 @@
  * See the file "license.terms" for information on usage and redistribution
  * of this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tkOldConfig.c,v 1.12.2.1 2005/08/16 16:18:59 dkf Exp $
+ * RCS: @(#) $Id: tkOldConfig.c,v 1.12.2.2 2005/09/14 23:27:02 dkf Exp $
  */
 
 #include "tkPort.h"
@@ -45,6 +45,8 @@ static CONST char *	FormatConfigValue _ANSI_ARGS_((Tcl_Interp *interp,
 			    Tk_Window tkwin, Tk_ConfigSpec *specPtr,
 			    char *widgRec, char *buffer,
 			    Tcl_FreeProc **freeProcPtr));
+static Tk_ConfigSpec *	GetCachedSpecs _ANSI_ARGS_((Tcl_Interp *interp,
+			    const Tk_ConfigSpec *staticSpecs));
 static void		DeleteSpecCacheTable _ANSI_ARGS_((
 			    ClientData clientData, Tcl_Interp *interp));
 
@@ -62,9 +64,12 @@ static void		DeleteSpecCacheTable _ANSI_ARGS_((
  *	the interp's result will hold an error message.
  *
  * Side effects:
- *	The fields of widgRec get filled in with information
- *	from argc/argv and the option database.  Old information
- *	in widgRec's fields gets recycled.
+ *	The fields of widgRec get filled in with information from
+ *	argc/argv and the option database.  Old information in
+ *	widgRec's fields gets recycled. A copy of the spec-table is
+ *	taken with (some of) the char* *fields converted into Tk_Uid
+ *	fields; this copy will be released when *the interpreter
+ *	terminates.
  *
  *--------------------------------------------------------------
  */
@@ -91,10 +96,6 @@ Tk_ConfigureWidget(interp, tkwin, specs, argc, argv, widgRec, flags)
 				 * or else they are not considered. */
     int hateFlags;		/* If a spec contains any bits here, it's
 				 * not considered. */
-    Tk_ConfigSpec *cachedSpecPtr;
-    Tcl_HashTable *specCacheTablePtr;
-    Tcl_HashEntry *entryPtr;
-    int isNew;
 
     if (tkwin == NULL) {
 	/*
@@ -113,62 +114,10 @@ Tk_ConfigureWidget(interp, tkwin, specs, argc, argv, widgRec, flags)
     }
 
     /*
-     * Pass zero: see if we've got a build of the config for this interpreter.
+     * Get the build of the config for this interpreter.
      */
 
-    specCacheTablePtr = (Tcl_HashTable *)
-	    Tcl_GetAssocData(interp, "tkConfigSpec.threadTable", NULL);
-    if (specCacheTablePtr == NULL) {
-	specCacheTablePtr = (Tcl_HashTable *) ckalloc(sizeof(Tcl_HashTable));
-	Tcl_InitHashTable(specCacheTablePtr, TCL_ONE_WORD_KEYS);
-	Tcl_SetAssocData(interp, "tkConfigSpec.threadTable",
-		DeleteSpecCacheTable, (ClientData) specCacheTablePtr);
-    }
-    entryPtr = Tcl_CreateHashEntry(specCacheTablePtr, (char *) specs, &isNew);
-    if (isNew) {
-	unsigned int entrySpace = sizeof(Tk_ConfigSpec);
-
-	/*
-	 * OK, no working copy in this interpreter so copy. Need to work out
-	 * how much space to allocate first.
-	 */
-
-	for (specPtr = specs; specPtr->type != TK_CONFIG_END; specPtr++) {
-	    entrySpace += sizeof(Tk_ConfigSpec);
-	}
-
-	/*
-	 * Now allocate our working copy's space and copy over the contents
-	 * from the master copy.
-	 */
-
-	cachedSpecPtr = (Tk_ConfigSpec *) ckalloc(entrySpace);
-	memcpy((void *) cachedSpecPtr, (void *) specs, entrySpace);
-	Tcl_SetHashValue(entryPtr, (ClientData) cachedSpecPtr);
-
-	/*
-	 * Finally, go through and replace database names, database classes
-	 * and default values with Tk_Uids. This is the bit that has to be
-	 * per-thread.
-	 */
-
-	for (specPtr=cachedSpecPtr; specPtr->type!=TK_CONFIG_END; specPtr++) {
-	    if (specPtr->argvName != NULL) {
-		if (specPtr->dbName != NULL) {
-		    specPtr->dbName = Tk_GetUid(specPtr->dbName);
-		}
-		if (specPtr->dbClass != NULL) {
-		    specPtr->dbClass = Tk_GetUid(specPtr->dbClass);
-		}
-		if (specPtr->defValue != NULL) {
-		    specPtr->defValue = Tk_GetUid(specPtr->defValue);
-		}
-	    }
-	    specPtr->specFlags &= ~TK_CONFIG_OPTION_SPECIFIED;
-	}
-    } else {
-	cachedSpecPtr = (Tk_ConfigSpec *) Tcl_GetHashValue(entryPtr);
-    }
+    specs = GetCachedSpecs(interp, specs);
 
     /*
      * Pass one:  scan through all of the arguments, processing those
@@ -183,8 +132,7 @@ Tk_ConfigureWidget(interp, tkwin, specs, argc, argv, widgRec, flags)
 	} else {
 	    arg = *argv;
 	}
-	specPtr = FindConfigSpec(interp, cachedSpecPtr, arg, needFlags,
-		hateFlags);
+	specPtr = FindConfigSpec(interp, specs, arg, needFlags, hateFlags);
 	if (specPtr == NULL) {
 	    return TCL_ERROR;
 	}
@@ -224,7 +172,7 @@ Tk_ConfigureWidget(interp, tkwin, specs, argc, argv, widgRec, flags)
      */
 
     if (!(flags & TK_CONFIG_ARGV_ONLY)) {
-	for (specPtr=cachedSpecPtr; specPtr->type!=TK_CONFIG_END; specPtr++) {
+	for (specPtr=specs; specPtr->type!=TK_CONFIG_END; specPtr++) {
 	    if ((specPtr->specFlags & TK_CONFIG_OPTION_SPECIFIED)
 		    || (specPtr->argvName == NULL)
 		    || (specPtr->type == TK_CONFIG_SYNONYM)) {
@@ -675,9 +623,6 @@ Tk_ConfigureInfo(interp, tkwin, specs, widgRec, argvName, flags)
     int needFlags, hateFlags;
     char *list;
     char *leader = "{";
-    Tcl_HashTable *specCacheTablePtr;
-    Tcl_HashEntry *entryPtr;
-    Tk_ConfigSpec *cachedSpecPtr;
 
     needFlags = flags & ~(TK_CONFIG_USER_BIT - 1);
     if (Tk_Depth(tkwin) <= 1) {
@@ -687,14 +632,10 @@ Tk_ConfigureInfo(interp, tkwin, specs, widgRec, argvName, flags)
     }
 
     /*
-     * Get the build of the config for this interpreter. Assumes that
-     * it already exists, but that assumption was embedded anyway.
+     * Get the build of the config for this interpreter.
      */
 
-    specCacheTablePtr = (Tcl_HashTable *)
-	    Tcl_GetAssocData(interp, "tkConfigSpec.threadTable", NULL);
-    entryPtr = Tcl_FindHashEntry(specCacheTablePtr, (char *) specs);
-    cachedSpecPtr = (Tk_ConfigSpec *) Tcl_GetHashValue(entryPtr);
+    specs = GetCachedSpecs(interp, specs);
 
     /*
      * If information is only wanted for a single configuration
@@ -703,8 +644,7 @@ Tk_ConfigureInfo(interp, tkwin, specs, widgRec, argvName, flags)
 
     Tcl_SetResult(interp, (char *) NULL, TCL_STATIC);
     if (argvName != NULL) {
-	specPtr = FindConfigSpec(interp, cachedSpecPtr, argvName, needFlags,
-		hateFlags);
+	specPtr = FindConfigSpec(interp, specs, argvName, needFlags,hateFlags);
 	if (specPtr == NULL) {
 	    return TCL_ERROR;
 	}
@@ -719,7 +659,7 @@ Tk_ConfigureInfo(interp, tkwin, specs, widgRec, argvName, flags)
      * their information.
      */
 
-    for (specPtr = cachedSpecPtr; specPtr->type != TK_CONFIG_END; specPtr++) {
+    for (specPtr = specs; specPtr->type != TK_CONFIG_END; specPtr++) {
 	if ((argvName != NULL) && (specPtr->argvName != argvName)) {
 	    continue;
 	}
@@ -990,9 +930,6 @@ Tk_ConfigureValue(interp, tkwin, specs, widgRec, argvName, flags)
     Tcl_FreeProc *freeProc;
     CONST char *result;
     char buffer[200];
-    Tcl_HashTable *specCacheTablePtr;
-    Tcl_HashEntry *entryPtr;
-    Tk_ConfigSpec *cachedSpecPtr;
 
     needFlags = flags & ~(TK_CONFIG_USER_BIT - 1);
     if (Tk_Depth(tkwin) <= 1) {
@@ -1001,13 +938,13 @@ Tk_ConfigureValue(interp, tkwin, specs, widgRec, argvName, flags)
 	hateFlags = TK_CONFIG_MONO_ONLY;
     }
 
-    specCacheTablePtr = (Tcl_HashTable *)
-	    Tcl_GetAssocData(interp, "tkConfigSpec.threadTable", NULL);
-    entryPtr = Tcl_FindHashEntry(specCacheTablePtr, (char *) specs);
-    cachedSpecPtr = (Tk_ConfigSpec *) Tcl_GetHashValue(entryPtr);
+    /*
+     * Get the build of the config for this interpreter.
+     */
+
+    specs = GetCachedSpecs(interp, specs);
 
-    specPtr = FindConfigSpec(interp, cachedSpecPtr, argvName, needFlags,
-	    hateFlags);
+    specPtr = FindConfigSpec(interp, specs, argvName, needFlags, hateFlags);
     if (specPtr == NULL) {
 	return TCL_ERROR;
     }
@@ -1103,6 +1040,114 @@ Tk_FreeOptions(specs, widgRec, display, needFlags)
 /*
  *--------------------------------------------------------------
  *
+ * GetCachedSpecs --
+ *
+ *Returns a writable per-interpreter (and hence thread-local) copy of
+ *the given spec-table with (some of) the char* fields converted into
+ *Tk_Uid fields; this copy will be released when the interpreter
+ *terminates (during AssocData cleanup).
+ *
+ * Results:
+ *A pointer to the copied table.
+ *
+ * Notes:
+ *The conversion to Tk_Uid is only done the first time, when the table
+ *copy is taken. After that, the table is assumed to have Tk_Uids where
+ *they are needed. The time of deletion of the caches isn't very
+ *important unless you've got a lot of code that uses Tk_ConfigureWidget
+ *(or *Info or *Value} when the interpreter is being deleted.
+ *
+ *--------------------------------------------------------------
+ */
+
+static Tk_ConfigSpec *
+GetCachedSpecs(interp, staticSpecs)
+    Tcl_Interp *interp;		/* Interpreter in which to store the cache. */
+    const Tk_ConfigSpec *staticSpecs;
+				/* Value to cache a copy of; it is also used
+				 * as a key into the cache. */
+{
+    Tk_ConfigSpec *cachedSpecs;
+    Tcl_HashTable *specCacheTablePtr;
+    Tcl_HashEntry *entryPtr;
+    int isNew;
+
+    /*
+     * Get (or allocate if it doesn't exist) the hash table that the writable
+     * copies of the widget specs are stored in. In effect, this is
+     * self-initializing code.
+     */
+
+    specCacheTablePtr = (Tcl_HashTable *)
+	    Tcl_GetAssocData(interp, "tkConfigSpec.threadTable", NULL);
+    if (specCacheTablePtr == NULL) {
+	specCacheTablePtr = (Tcl_HashTable *) ckalloc(sizeof(Tcl_HashTable));
+	Tcl_InitHashTable(specCacheTablePtr, TCL_ONE_WORD_KEYS);
+	Tcl_SetAssocData(interp, "tkConfigSpec.threadTable",
+		DeleteSpecCacheTable, (ClientData) specCacheTablePtr);
+    }
+
+    /*
+     * Look up or create the hash entry that the constant specs are mapped to,
+     * which will have the writable specs as its associated value.
+     */
+
+    entryPtr = Tcl_CreateHashEntry(specCacheTablePtr, (char *) staticSpecs,
+	    &isNew);
+    if (isNew) {
+	unsigned int entrySpace = sizeof(Tk_ConfigSpec);
+	const Tk_ConfigSpec *staticSpecPtr;
+	Tk_ConfigSpec *specPtr;
+
+	/*
+	 * OK, no working copy in this interpreter so copy. Need to work out
+	 * how much space to allocate first.
+	 */
+
+	for (staticSpecPtr=staticSpecs; staticSpecPtr->type!=TK_CONFIG_END;
+		staticSpecPtr++) {
+	    entrySpace += sizeof(Tk_ConfigSpec);
+	}
+
+	/*
+	 * Now allocate our working copy's space and copy over the contents
+	 * from the master copy.
+	 */
+
+	cachedSpecs = (Tk_ConfigSpec *) ckalloc(entrySpace);
+	memcpy((void *) cachedSpecs, (void *) staticSpecs, entrySpace);
+	Tcl_SetHashValue(entryPtr, (ClientData) cachedSpecs);
+
+	/*
+	 * Finally, go through and replace database names, database classes
+	 * and default values with Tk_Uids. This is the bit that has to be
+	 * per-thread.
+	 */
+
+	for (specPtr=cachedSpecs; specPtr->type!=TK_CONFIG_END; specPtr++) {
+	    if (specPtr->argvName != NULL) {
+		if (specPtr->dbName != NULL) {
+		    specPtr->dbName = Tk_GetUid(specPtr->dbName);
+		}
+		if (specPtr->dbClass != NULL) {
+		    specPtr->dbClass = Tk_GetUid(specPtr->dbClass);
+		}
+		if (specPtr->defValue != NULL) {
+		    specPtr->defValue = Tk_GetUid(specPtr->defValue);
+		}
+	    }
+	    specPtr->specFlags &= ~TK_CONFIG_OPTION_SPECIFIED;
+	}
+    } else {
+	cachedSpecs = (Tk_ConfigSpec *) Tcl_GetHashValue(entryPtr);
+    }
+
+    return cachedSpecs;
+}
+
+/*
+ *--------------------------------------------------------------
+ *
  * DeleteSpecCacheTable --
  *
  *	Delete the per-interpreter copy of all the Tk_ConfigSpec tables which