View Issue Details

IDProjectCategoryView StatusLast Update
0000558LDMud 3.3Compilation, Installationpublic2009-01-16 13:52
Reporterzesstra Assigned Tozesstra  
PrioritylowSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platformi686OSGNU/LinuxOS Version4.0
Product Version3.3 
Target Version3.3.719Fixed in Version3.3.719 
Summary0000558: Too large ITABLE_SIZE leads to errors finding identifiers in the ident_table.
Descriptionfree_shared_identifier() in the lexer doesn't find identifiers in the ident_table if the ident_table is too big.
I made some experiments with its size (config.h.in states it should be chosen several times smaller or equal to the string hash table size). Choosing some value well above 32768 leads the testsuite to fail completely. Note that choosing the size equal to HTABLE_SIZE does not work right now if the HTABLE_SIZE is 65536. ;-)

The reason for this is the fact that ident_s->hash is of type short:
    short hash; /* Hashvalue of this identifier */

We should either change the type here (and check stuff like explicit casts of the hash to short, as in lookfor_shared_identifier()) or enforce an ITABLE_SIZE of <= 32768 by some precompiler check and update the comment in config.h.in.
Steps To ReproduceSet ITABLE_SIZE in config.h to 65536, recompile and execute the testsuite.
Additional InformationTestsuite log:
[...]
2008.08.07 14:41:26 free_shared_identifier: name 'str' not found!
2008.08.07 14:41:26 Current object was <null>
2008.08.07 14:41:27 Failed to load master object 't-mantis'.
Test t-mantis.c FAILED.
The following tests FAILED:

    t-0000398
    t-0000486.c
    t-0000488.c
    t-0000523.c
    t-0000524.c
    t-0000532.c
    t-0000534.c
    t-0000548
    t-030925
    t-040413
    t-041124
    t-efuns.c
    t-errors.c
    t-mantis.c
TagsNo tags attached.

Activities

zesstra

2009-01-08 03:56

administrator   ~0000854

Any preferences for dealing with this? Enforcing a max of SHORT_MAX would be the easiest, 32k hash chains sound like a lot.
But on the other hand the ident_table seems to be a global table of all identifiers in the game (or am I wrong here?). A clean solution would be to use a specific hash type as well, maybe with an automatic selection of the underlying basic type according to ITABLE_SIZE?

Gnomi

2009-01-08 04:19

manager   ~0000856

The ident_table contains all global identifiers (efuns and permanent defines) as well as the identifiers of a compiled program that are removed from the ident_table after compilation. 32k seems enough for now, so I'm fine with both solutions.

zesstra

2009-01-08 04:46

administrator   ~0000859

Ok. Then I will limit ITABLE_SIZE for 3.3.x and have a second look concerning hashes and hash types in 3.5.x during harmonizing all the other hashes. (0000564, 0000518).

zesstra

2009-01-16 13:37

administrator   ~0000907

Ok, the immediate issue should be solved by r2494.
I will probably open a separate bug in 3.5 for the issue of changing the hash.

zesstra

2009-01-16 13:52

administrator   ~0000908

I just thought again about the ident_t hashes. identhash() already uses the hash functions from hash.c and as 32k hashchains should be really sufficient for the fore-seeable future, there is IMHO no real demand for changing things, as long as the limit is documented. I added it to /doc/driver/limitations.

Issue History

Date Modified Username Field Change
2008-08-07 07:09 zesstra New Issue
2009-01-08 03:56 zesstra Note Added: 0000854
2009-01-08 03:57 zesstra Status new => assigned
2009-01-08 03:57 zesstra Assigned To => zesstra
2009-01-08 04:19 Gnomi Note Added: 0000856
2009-01-08 04:46 zesstra Note Added: 0000859
2009-01-08 04:47 zesstra Relationship added child of 0000564
2009-01-08 05:05 zesstra Target Version => 3.3.719
2009-01-16 13:37 zesstra Note Added: 0000907
2009-01-16 13:37 zesstra Status assigned => resolved
2009-01-16 13:37 zesstra Fixed in Version => 3.3.719
2009-01-16 13:37 zesstra Resolution open => fixed
2009-01-16 13:52 zesstra Note Added: 0000908
2009-01-16 13:52 zesstra Relationship deleted child of 0000564