Review: Disapprove

There are several problems:

1. The new string pool is not thread-safe (the current one is)

2. The check made in ~StringPool() must be implemented in the new pool as well

3. Most of the time, the strings that are inserted in the pool are "char *" 
strings (for example, during doc loading, libxml gives chrar* strings to the 
loader). The StringPool::insertc() method is there to optimize this case: it 
will avoid the creation of a zstring (and the resulting string copy) if the 
string is already in the pool, which is the most common case. This optimization 
is lost with the new pool.

4. I think there is a bug in BasicItemFactory::createAnyURI(). The AnyUriItem 
constructor does a destructive assignment (calls take() on the input zstring). 
So, if the expression *theUriPool->insert(value).first returns a reference to 
the zstring that is in the pool, that zstring will be cleared.

I am not against this change in principle, but at the same time I don't 
consider it important enough to spend much time on it. So, I would prefer if 
this is simply dropped.
Your team Zorba Coders is subscribed to branch lp:zorba.

Mailing list:
Post to     :
Unsubscribe :
More help   :

Reply via email to