Re: [PATCH] g_type_add_interface_check()

[Tim Janik <timj gtk org>]

On Thu, 25 Sep 2003, Owen Taylor wrote:

> Here is a patch to add, basically as described in your last mail:
>
> typedef void (*GTypeInterfaceCheckFunc) (gpointer func_data,
>                                          gpointer g_iface);
>
> void g_type_add_interface_check    (gpointer                check_data,
>                                     GTypeInterfaceCheckFunc check_func);
> void g_type_remove_interface_check (gpointer                check_data,
>                                     GTypeInterfaceCheckFunc chec_func);
>
> Notes:
>
>  - I removed the gboolean return from GTypeInterfaceCheckFunc, since
>    I couldn't see anything useful for GObject to do with it; if
>    the check fails, GObject can't gracefully not initialize the object.

the bool return was a c-n-p left over, meant to be void.

>  - I move the setting of entry->init_state in gobject.c as you suggested,
>    since it simplified the logic and cleaned things up.

s/gobject.c/gtype.c/

>    However, I did not add reentrancy guards as you suggested, because I
>    don't think reentrancy is possible.
>
>    type_iface_vtable_iface_init_Wm() is called in two circumstances:
>
>    - When initializing interfaces after calling class_init()
>    - When an interface is added to a class that has already been initialized
>
>    But in no circumstances can either of these cases recurse - once an
>    interface has been initialized for a particular class, it has been
>    initialized and can't be initialized again until the class is unloaded.

that's with the current setup and isn't guarranteed to stay
for future changes. so tests or assertions on init_state wouldn't
hurt i think.

> @@ -1673,16 +1681,15 @@ type_iface_vtable_iface_init_Wm (TypeNod
>                                  TypeNode *node)
>  {
>    IFaceEntry *entry = type_lookup_iface_entry_L (node, iface);
> -  IFaceHolder *iholder;
> +  IFaceHolder *iholder = type_iface_peek_holder_L (iface, NODE_TYPE (node));
>    GTypeInterface *vtable = NULL;
> +  guint i;
>
> -  iholder = type_iface_peek_holder_L (iface, NODE_TYPE (node));
> -  if (!iholder)
> -    return;
> -
>    /* iholder->info should have been filled in by type_iface_vtable_base_init_Wm() */
> -  g_assert (iface->data && entry && iholder->info);
> +  g_assert (iface->data && entry && iholder && iholder->info);

puh, that was a non-obvious change. and it is not ok.
though, that's partly due to a bug in gtype.c which also needs to be fixed.
if in a class_init function a new interface is added to one of its
anchestors, the assumption you make here, entry->init_state==INITIALIZED
is not true, and the class doesn't have a holder info.

first, to fix the non-obviousness of your change, we need extra comments:

@type_class_init_Wm

>   if (node->data->class.class_init)
>     node->data->class.class_init (class, (gpointer) node->data->class.class_data);
>
>   G_WRITE_LOCK (&type_rw_lock);
>
>   node->data->class.init_state = IFACE_INIT;
>
>   /* finish initialize the interfaces through our holder info

+    * inherited interfaces are already init_state == INITIALIZED, because
+    * they either got setup in the above base_init loop, or during
+    * class_init from within type_iface_vtable_init_Wm() for this or
+    * an anchestor type.

>    */
>   i = 0;
>   while (TRUE)
>     {
>       entry = &CLASSED_NODE_IFACES_ENTRIES (node)[i];


@type_iface_vtable_iface_init_Wm

> /* Finishes what type_iface_vtable_base_init_Wm started by
>  * calling the interface init function.

+  * this function may only be called for types with their
+  * own interface holder info, i.e. types for which
+  * g_type_add_interface*() was called and not children thereof.

>  */
> static void
> type_iface_vtable_iface_init_Wm (TypeNode *iface,
>                                  TypeNode *node)
> {
>   IFaceEntry *entry = type_lookup_iface_entry_L (node, iface);
>   IFaceHolder *iholder = type_iface_peek_holder_L (iface, NODE_TYPE (node));
>   GTypeInterface *vtable = NULL;
>   guint i;
>
>   /* iholder->info should have been filled in by type_iface_vtable_base_init_Wm() */
>   g_assert (iface->data && entry && iholder && iholder->info);

+   g_assert (entry->init_state == IFACE_INIT); /* assert prior base_init() */

>
>   entry->init_state = INITIALIZED;


secondly, type_iface_vtable_init_Wm() needs to be fixed according to
the above comment. that is, after we did base_init and iface_init
in type_iface_vtable_init_Wm(), it needs to adjust the iface entries
of all its child classes, according to their intialization state,
i'm currently working on a patch for that.

>
> Regards,
> 						Owen
>

---
ciaoTJ