Rusty Russell [Thu, 16 Dec 2010 05:33:41 +0000 (16:03 +1030)]
typesafe_cb: Fix warnings with gcc-4.5:
Test compiled with warnings:
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c: In function ‘main’:
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:18:2: warning: taking address of expression of type ‘void’
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:18:2: warning: taking address of expression of type ‘void’
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:18:2: warning: taking address of expression of type ‘void’
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:18:2: warning: taking address of expression of type ‘void’
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:18:2: warning: taking address of expression of type ‘void’
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:18:2: warning: taking address of expression of type ‘void’
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:18:2: warning: taking address of expression of type ‘void’
/home/rusty/devel/cvs/ccan/ccan/typesafe_cb/test/compile_ok-typesafe_cb-NULL.c:19:2: warning: taking address of expression of type ‘void’
Rusty Russell [Mon, 13 Dec 2010 08:15:25 +0000 (18:45 +1030)]
ccanlint: report valgrind "definite" leaks.
This is complicated by valgrind's limited options, and our desire not to run
valgrind twice (it's already the slowest part of the tests).
Ideally I'd like a different error code for each kind of error. I
could parse and pretty-print the XML output, but instead I just parse
the human-readable (which is fragile).
Ronnie Sahlberg [Wed, 8 Dec 2010 01:00:35 +0000 (12:00 +1100)]
rb_tree: fix trbt_delete
trbt_delete32() was broken and caused SEGV as soon as you tried to
delete an object from a tree.
Rework trbt_delete32() to instead just call talloc_free() instread of trying
to call delete_node() directly.
This makes the "from_destructor" argument to delete_node() redundant
so that parameter is removed too.
Signed-off-by: Ronnie Sahlberg <sahlberg@lenovo-laptop.(none)>
Rusty Russell [Wed, 8 Dec 2010 00:34:03 +0000 (11:04 +1030)]
ccanlint: fix compile on x86-64
cc -g -Wall -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -I. -MD -Werror -c -o tools/ccanlint/tests/examples_run.o tools/ccanlint/tests/examples_run.c
cc1: warnings being treated as errors
tools/ccanlint/tests/examples_run.c: In function ‘scan_forv’:
tools/ccanlint/tests/examples_run.c:37: warning: passing argument 2 of ‘__builtin_va_copy’ discards qualifiers from pointer target type
tools/ccanlint/tests/examples_run.c:43: warning: passing argument 4 of ‘scan_forv’ from incompatible pointer type
tools/ccanlint/tests/examples_run.c:52: warning: passing argument 4 of ‘scan_forv’ from incompatible pointer type
tools/ccanlint/tests/examples_run.c:60: warning: passing argument 4 of ‘scan_forv’ from incompatible pointer type
tools/ccanlint/tests/examples_run.c: In function ‘scan_for’:
tools/ccanlint/tests/examples_run.c:78: warning: passing argument 4 of ‘scan_forv’ from incompatible pointer type
make: *** [tools/ccanlint/tests/examples_run.o] Error 1
It really doesn't like constifying a va_arg, so remove the const declaration.
Right-shifting signed integers in undefined; indeed it seems that on
AIX with their compiler, doing a 30-bit shift on (INT_MAX-200) gives
0, not 1 as we might expect (THIS WAS WRONG, REAL FIX LATER).
The obvious fix is to make id and oid unsigned: l (level count) is also
logically unsigned.
(Note: Samba doesn't generally get to ids > 1 billion, but ctdb does)
Reported-by: Chris Cowan <cc@us.ibm.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Author: Rusty Russell <rusty@rustorp.com.au>
Date: Thu Jun 10 13:27:51 2010 -0700
Since idtree assigns sequentially, it rarely reaches high numbers.
But such numbers can be forced with idr_get_new_above(), and that
reveals two bugs:
1) Crash in sub_remove() caused by pa array being too short.
2) Shift by more than 32 in _idr_find(), which is undefined, causing
the "outside the current tree" optimization to misfire and return NULL.
Rusty Russell [Wed, 1 Dec 2010 13:13:37 +0000 (23:43 +1030)]
tdb2: hash chaining
If we get enough hash collisions, we can run out of hash bits; this almost
certainly is caused by a deliberate attempt to break the tdb (hash bombing).
Implement chained records for this case; they're slow but will keep the
rest of the database functioning.
Rusty Russell [Wed, 1 Dec 2010 13:11:26 +0000 (23:41 +1030)]
tdb2: use magic freetable value rather than different magic for coalescing
We have to unlock during coalescing, so we mark records specially to indicate
to tdb_check that they're not on any list, and to prevent other coalescers
from grabbing them.
Use a special free list number, rather than a new magic.
Rusty Russell [Wed, 1 Dec 2010 13:10:05 +0000 (23:40 +1030)]
tdb2: compare the extra 11 hash bits in the header.
We already have 10 hash bits encoded in the offset itself; we only get
here incorrectly about 1 time in 1000, so it's a pretty minor
optimization at best.
Nonetheless, we have the information, so let's check it before
accessing the key. This reduces the probability of a false keycmp by
another factor of 2000.
Rusty Russell [Wed, 1 Dec 2010 12:41:51 +0000 (23:11 +1030)]
tdb2: clean up logging
Logged errors should always set tdb->ecode before they are called, and
there's little reason to have a sprintf-style logging function since
we can do the formatting internally.
Change the tdb_log attribute to just take a "const char *", and create
a new tdb_logerr() helper which sets ecode and calls it. As a bonus,
mark it COLD so the compiler can optimize appropriately knowing that
it's unlikely to be invoked.
Rusty Russell [Wed, 1 Dec 2010 12:38:52 +0000 (23:08 +1030)]
tdb2: remove truncated bit.
There was an idea that we would use a bit to indicate that we didn't have
the full hash value; this would allow us to move records down when we
expanded a hash without rehashing them.
There's little evidence that rehashing in this case is particularly
expensive, so remove the bit. We use that bit simply to indicate that
an offset refers to a subhash instead.
Rusty Russell [Wed, 1 Dec 2010 12:34:50 +0000 (23:04 +1030)]
tdb2: remove tdb_get()
We have four internal helpers for reading data from the database:
1) tdb_read_convert() - read (and convert) into a buffer.
2) tdb_read_off() - read (and convert) and offset.
3) tdb_access_read() - malloc or direct access to the database.
4) tdb_get() - copy into a buffer or direct access to the database.
The last one doesn't really buy us anything, so remove it (except for
tdb_read_off/tdb_write_off, see next patch).
Rusty Russell [Tue, 23 Nov 2010 01:43:59 +0000 (12:13 +1030)]
tdb2: stricter ordering on expansion lock
It's problematic for transaction commit to get the expansion lock, but
in fact we always grab a hash lock before the transaction lock, so it
doesn't really need it (the transaction locks the entire database).
Assert that this is true, and fix up a few lowlevel tests where it wasn't.
Rusty Russell [Tue, 23 Nov 2010 01:37:29 +0000 (12:07 +1030)]
tdb2: allow nesting of read locks on top of write locks.
If we have a write lock and ask for a read lock, that's OK, but not the
other way around. tdb_nest_lock() allowed both, tdb_allrecord_lock() allowed
neither.
Rusty Russell [Mon, 22 Nov 2010 02:13:00 +0000 (12:43 +1030)]
tdb2: relax locking to allow two free list locks at once
As long as they are in descending order. This prevents the common case of:
1) Grab lock for bucket.
2) Remove entry from bucket.
3) Drop lock for bucket.
4) Grab lock for bucket for leftover.
5) Add leftover entry to bucket.
6) Drop lock for leftover bucket.
In particular it's quite common for the leftover bucket to be the same
as the entry bucket (when it's the largest bucket); if it's not, we are
no worse than before.
Rusty Russell [Wed, 1 Dec 2010 13:22:43 +0000 (23:52 +1030)]
tdb2: shrink free header from 32 to 24 bytes.
This reduces our minimum key+data length to 8 bytes; we do this by packing
the prev pointer where we used to put the flist pointer, and storing the
flist as an 8 bit index (meaning we can only have 256 free tables).
Note that this has a perverse result on the size of the database, as our
4-byte key and 4-byte data now fit perfectly in a minimal record, so
appeding causes us to allocate new records which are 50% larger,
since we detect growing.
Rusty Russell [Wed, 17 Nov 2010 09:55:41 +0000 (20:25 +1030)]
tdb2: get rid of zones
Zones were a bad idea. They mean we can't simply add stuff to the end
of the file (which transactions relied upon), and there's no good heuristic
in how to size them.
Rusty Russell [Mon, 15 Nov 2010 07:24:15 +0000 (17:54 +1030)]
tdb2: fix tdb_chainlock
We can't enlarge the lock without risking deadlock, so tdb_chainlock() can't
simply grab a one-byte lock; it needs to grab the lock we'd need to protect
the hash.
In theory, tdb_chainlock_read() could use a one-byte lock though.
Rusty Russell [Mon, 15 Nov 2010 07:25:45 +0000 (17:55 +1030)]
tdb2: fix coalesce race #3
When we're coalescing, we need to drop the lock on the current free list, as
we've enlarged the block and it may now belong in a different list.
Unfortunately (as shown by repeated tdbtorture -n 8) another coalescing run
can do the coalescing while we've dropped the lock. So for this case, we
use the TDB_COALESCING_MAGIC flag so it doesn't look free.
Rusty Russell [Mon, 15 Nov 2010 08:32:42 +0000 (19:02 +1030)]
tdb2: fix coalesce race #2
When we find a free block, we need to mark it as used before we drop the
free lock, even though we've removed it from the list. Otherwise the
coalescing code can find it.
This means handing the information we need down to lock_and_alloc, which
also means we know when we're grabbing a "growing" entry, and can relax
the heuristics about finding a good-sized block in that case.
Rusty Russell [Mon, 15 Nov 2010 07:26:57 +0000 (17:56 +1030)]
tdb2: hoist adjust_size
We're going to want it in get_free() in the next patch, so move it upwards.
Trivial changes, too: add to size before min length check, and rename growing
to want_extra.
Rusty Russell [Wed, 17 Nov 2010 09:59:13 +0000 (20:29 +1030)]
ccanlint: add ccanlint section to _info
This supersedes the previous Fails: section, into a more general set of
lines of form:
<testname> <option>...
With the special <option> "FAIL" to mean we know we fail this test.
We accept options to valgrind-tests; in particular tdb2 wants
--partial-loads-ok=yes passed to valgrind.
Rusty Russell [Wed, 10 Nov 2010 06:38:29 +0000 (17:08 +1030)]
ccanlint: tweak example compilation.
We used to prefer to build up examples using previous ones, but this broke
for tests which were to be executed. But list.h relied on the buildup
behavior, so return to that unless we see "// Given" comments indicating
an execution test.