On 03/05/15 09:09, Howard Chu wrote:

Hallvard Breien Furuseth wrote:

...

test001 still crashes if you insert assert(a->mv_size == sizeof(int) && b->mv_size == sizeof(int)); in mdb_cmp_int(). Using RE24 commit 98f70f1841f85d93 "ITS#8120 Move final...".

Not happening here, all tests pass with asserts enabled.

You just fixed it. Turns out your older patches to this ITS introduced that one.

...

Looking yet again at the doc, mdb_dbi_open():INTEGERKEY says "typically" sizeof(int/size_t), I can't see it requires these sizes. Maybe you copied that from a suggestion from me back when I asked for clarifications.

Probably. The point was to show the expected sizes without enumerating every possible C integer type that would work - e.g. unsigned int, unsigned long, off_t, pid_t, whatever. At some point you have to say "don't be an idiot."

Don't be an idiot. (a) then we'd just say "the architecture's native unsigned integer types" or something, it's no need to be so vague nor to drag in every typedef under the sun. And (b) that's not what the code does anyway. If I understand correctly this time, the doc should simply say that the two types size_t and unsigned int are supported. The "typically" is plain misleading.

That is, if mdb_node_search() shows the intent when it picks either int-sized or size_t-sized for IS_BRANCH. OTOH, another bug:

default_cmp pick cmp_int for INTEGERDUP|DUPFIXED and some other replaces md_dcmp = cmp_int with cmp_long, which makes no sense unless cmp_int is just a placeholder. Yet at least mdb_dcmp() calls that placeholder directly without checking the data size. And if it's just a placeholder, how about instead setting dcmp to something which just abort()s, to make sure the bug of not replacing it is caught?

...

"keys must all be of the same size" in the doc is unclear. I gather it should be "... in the DB's lifetime". But it can be read as "...at a given time": Write size_t-sized integers, empty the DB, then write int-sized integers. That does not work, since me_dbxs[].md_dcmp is updated.

Don't be an idiot. If no time is stated, then it's For All Time. Anything not called out specifically is, by definition, non-specific.

I can't spot the idiocy of expecting a property which springs dynamically into life, to dynamically go away when the data which caused it is gone. In particular since that's how LMDB alignment works. Anyway, that seems to be what happens after latest commit, so maybe it doesn't matter anymore.

Maybe the DB flags should include log2(integer sizes) in 2 2-bit fields.