[Bug 24618] [0.11] add TpStringPool, a refcounted pool of strings

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=24618

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |review-

--- Comment #2 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-04-08 11:04:07 PDT ---
> +/* Dynamic handle repo */
...
> + * TpStringPoolClass:
> + *
> + * The class of a dynamic handle repository. The contents of the struct
... (etc.)

Not true :-)

> +    /* Map GUINT_TO_POINTER(handle) -> (TpStringPoolHandlePriv *) */
> +    GHashTable *handle_to_priv;

I'd prefer the terminology in this string pool to be something other than
"handle". I'm not sure what a good word is, though; perhaps "number"?

I think it'd be worth having a dual API in terms of numbers and strings, like
the way GObject qdata has g_object_get_data *and* g_object_get_data_by_id.

> +    /* Normalization function */
> +    TpStringPoolNormalizeFunc normalize_function;

I still don't think this string pool is the right place for normalization, and
I'd have left it in the dynamic handle repo.

Note, in particular, that this normalize function doesn't have the same
signature as the one for the dynamic handle repo!

If you didn't put the normalization function and the normalization context in
the string pool, then it wouldn't need properties, which would make it much
simpler.

The documentation you cut and pasted into the string pool makes no sense for a
generic string pool.

If the normalization stayed in the dynamic handle repo, then the string pool
would just have a function equivalent to tp_string_pool_lookup_exact (but it
should be called tp_string_pool_lookup_by_number or something), and wouldn't
need a function equivalent to the current tp_string_pool_lookup (which involves
a malloc/free cycle).

In the TpDynamicHandleRepo reimplementation, you don't seem to do anything with
the normalization function - as far as I can see, it'll never be called. This
would break Gabble, a lot.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.