Game Development Community

Major Bug: FindObject: StringTable Lookup

by Prairie Games · in Torque Game Engine · 10/23/2002 (10:35 pm) · 11 replies

In simManager.cc (and simBase.cc), the code for FindObject may use:

StringTableEntry stName = StringTable->lookupn(name, len);

StringTable->lookupn() is ONLY used in the SimObject findObject() routines...


This will match something like WorldLoginDlg to WorldLoginDlg.gui because of strncmp... it could and will match an object named foo to an object named fooBar.. based on how long the name is.. when the stringTable entry is inserted... and the calculated hash value... bad bad bad...

I had to rename my Object to SWorldLoginDlg ... and then decided to track down why it named as WorldLoginDlg failed...

in:

stringTable.cc

Replace these lines:

//--------------------------------------
StringTableEntry _StringTable::lookupn(const char* val, S32 len, const bool  caseSens)
{
   Node **walk, *temp;
   U32 key = hashStringn(val, len);
   walk = &buckets[key % numBuckets];
   while((temp = *walk) != NULL)   {
      if(caseSens && !dStrncmp(temp->val, val, len))
            return temp->val;
      else if(!caseSens && !dStrnicmp(temp->val, val, len))
         return temp->val;
      walk = &(temp->next);
   }
   return NULL;
}

[b]with:[/b]

[code]

//--------------------------------------
//I am used in exclusivly in finding SimObjects
//Unlike dStrncmp() I match only when 
//len==dStrlen(stringtableentry)

StringTableEntry _StringTable::lookupn(const char* val, S32 len, const bool  caseSens)
{
   Node **walk, *temp;
   U32 key = hashStringn(val, len);
   walk = &buckets[key % numBuckets];
   while((temp = *walk) != NULL)   {
      if(caseSens && !dStrncmp(temp->val, val, len) && temp->val[len] == 0)
            return temp->val;
      else if(!caseSens && !dStrnicmp(temp->val, val, len) && temp->val[len] == 0)
         return temp->val;
      walk = &(temp->next);
   }
   return NULL;
}


-J
NextNext Thread
PrevPrevious Thread
More from Torque Game Engine


Torque Owner Daniel Neilsen
#1
10/23/2002 (11:15 pm)
Hey Josh,
Lets say the wrong thing was found by the lookup, would this output something to the console?

Reason I ask is I have been getting the occasionally error in the console in my game that sounds very much like this.

I might try this fix

Torque 3D Owner Robert Blanchet Jr.
#2
11/01/2002 (12:51 am)
This actually causes more problems then it solves.
You'll find that if you try to recurse through the Mission Scripts (*.mis) you won't be able to call nameToID correctly on the SimGroups and SimSets if the you go deeper down the hierarchy.

Torque Owner Prairie Games
Prairie Games, Inc.
#3
11/01/2002 (5:35 am)
Actually I did run into a problem with PlayerDropPoints with this... in my mad dash I forgot to go back and see why...

EDIT:

OK... fixed... see above...

-J

Torque Owner D Player
#4
11/01/2002 (7:08 am)
A few comments,

1. After looking through the code, it seems that your fix does exactly the same thing less efficiently. Did I miss something?
2. You seem to have a buffer overflow if len == 256
3. You should use a local buffer instead of a static buffer for efficiency. Saves on page swaps and reduces memory overhead.
4. I expect that matching WorldLoginDlg to WorldLoginDlg.gui is absolutely intentional, although it's the sort of tradeoff that ends up causing more problems than it solves. It's probably done to allow loading "XYZ" to load "XYZ.png" or "XYZ.bmp". So, quite a bit of code/script probably relies on this feature, and not all of them may be obvious.

If you want to add this hack, try adding && name[len] != '.' to the for statement (if you don't see where, you probably shouldn't be messing with this).

Joshua, I don't mean to flame you here but this really does seem wrong.

Torque Owner Prairie Games
Prairie Games, Inc.
#5
11/01/2002 (7:40 am)
As I commented that wasn't a great fix wrt code style...improved fix above...

On your comments,

1) The original used stringTable->lookupn()
2) nope... was an assertion that len < 256
3) good point.. moot now
4) Those functions are only for finding a specific SimObject... and there is no pattern matching...

I wouldn't call this a hack... matching an object named foo to one named fooBar is bad bad bad...

Frankly, I am surprised such a major bug exists with SimObject lookups!

-J

Torque Owner D Player
#6
11/01/2002 (8:47 am)
Joshua,

You're right that was a bug. I knew something seemed wrong about the whole deal. Perhaps this would work faster though...

StringTableEntry _StringTable::lookupn(const char* val, S32 len, const bool  caseSens)
{
   Node **walk, *temp;
   U32 key = hashStringn(val, len);
   walk = &buckets[key % numBuckets];
   while((temp = *walk) != NULL)   {
      if(caseSens && !dStrncmp(temp->val, val, len) && temp->val[len] == 0)
            return temp->val;
      else if(!caseSens && !dStrnicmp(temp->val, val, len) && temp->val[len] == 0)
         return temp->val;
      walk = &(temp->next);
   }
   return NULL;
}

Torque Owner D Player
#7
11/01/2002 (9:12 am)
Joshua mentioned that lookupn is not a good name for the modified function. May I suggest a global replace s/lookupn/lookupexactn/ ?

Torque 3D Owner Robert Blanchet Jr.
#8
11/01/2002 (9:29 am)
So this modified code works now?

Torque Owner Prairie Games
Prairie Games, Inc.
#9
11/01/2002 (9:47 am)
More than likely...

There shouldn't be any quirks... other than the fact lookupn() perhaps sounds a bit like strncmp() though they behave slightly differently...

-J

Torque Owner D Player
#10
11/01/2002 (9:50 am)
Alright you got me into this function so here's my final answer:

StringTableEntry _StringTable::lookupexactn(const char* val, S32 len, bool caseSens)
{
	Node *walk;
	U32 key	= hashStringn(val, len);
	walk	= buckets[key % numBuckets];
	while(walk != NULL)
	{
		if(caseSens)
		{
			if(!dStrncmp(walk->val, val, len) && walk->val[len] == 0)
				return walk->val;
		}
		else
			if(!dStrnicmp(walk->val, val, len) && walk->val[len] == 0)
				return walk->val;
		walk = walk->next;
	}
	return NULL;
}

Now I can think about something else.

Torque Owner Mark Owen
#11
01/05/2004 (4:57 pm)
For what its worth... the 1_2_0 code contains exactly the code posted on 2002 Oct 23 but not the later revisions to it.