Greg Stein <gstein@lyra.org> writes:
> So the question becomes: wtf was NULL passed? I'm wary of anything that is
> using malloc rather than a pool. IMO, we should always use a pool and never
> fall back to malloc. Consider: who is calling free() on that thing?
Don't fret --- we are of one mind on that issue.
The documentation for svn_fs_parse_id reads:
Allocate the parsed ID in POOL. If POOL is zero, malloc the ID; we
need this in certain cases where we can't pass in a pool, but it's
generally best to use a pool whenever possible. */
I've added the following comment to nodes-table.c, above
compare_nodes_keys, which is the only place where we use this:
NOTE WELL: this function and its helpers use `malloc' to get space
for the parsed node revision ID's. In general, we try to use pools
for everything in Subversion, but in this case it's not practical.
Berkeley DB doesn't provide any way to pass a baton through to the
btree comparison function. Even if it did, since Berkeley DB needs
to invoke the comparison function at pretty arbitrary times, you'd
have to pass the baton to almost every Berkeley DB operation. You
could stuff a pool pointer in a global variable, but then you'd
have to make sure the pool was up to date before every Berkeley DB
operation; you'd surely forget, leading to crashes... Using malloc
is more maintainable. Since the comparison function isn't allowed
to signal an error anyway, the need for pools isn't quite as urgent
as in other code, but we still need to take care. */
Received on Sat Oct 21 14:36:23 2006