diff mbox

[RFC,v0,5/8] object: make interfaces concrete

Message ID 4FD89604.7050606@codemonkey.ws
State New
Headers show

Commit Message

Anthony Liguori June 13, 2012, 1:30 p.m. UTC 
On 06/13/2012 08:02 AM, Paolo Bonzini wrote:
> Il 13/06/2012 14:59, Andreas Färber ha scritto:
>> Ouch! One can argue that's still not recursive, but what matters more
>> this borks Anthony's in-place object_initialize() concept.
>>
>> Two solutions come to mind:
>> * allocate the interfaces as part of object_new() beyond instance_size
>
> That won't work if you initialize in place, because you cannot allocate
> the room for the interface.  It is possible to put Interface objects
> explicitly in the class, and pass an offset when registering the type so
> that they can be initialized in place.
>
> But I still think we're fighting windmills...

There's no problem in my mind with allocating interfaces on the heap.  in-place 
initialization is a readability thing, it's not a memory management thing.  It's 
so you can see that:

struct PIIX3 {
   ...

   RTCState rtc;
   APICState *apic;
};

'rtc' is a child and 'apic' is a link.

Anyway, I don't like the idea of making interfaces concrete.  That means that a 
user could directly instantiate an interface which doesn't make a lot of sense.

Here's a different solution.  This has not been even compile tested but I think 
the concept is sound.

Regards,

Anthony Liguori

>
> Paolo
>
>

Comments

Paolo Bonzini June 13, 2012, 1:33 p.m. UTC | #1 
Il 13/06/2012 15:30, Anthony Liguori ha scritto:
> Anyway, I don't like the idea of making interfaces concrete.  That means
> that a user could directly instantiate an interface which doesn't make a
> lot of sense.

Concrete doesn't mean "instantiatable by the user".  It means
"instantiatable period".

Paolo
Anthony Liguori June 13, 2012, 1:36 p.m. UTC | #2 
On 06/13/2012 08:33 AM, Paolo Bonzini wrote:
> Il 13/06/2012 15:30, Anthony Liguori ha scritto:
>> Anyway, I don't like the idea of making interfaces concrete.  That means
>> that a user could directly instantiate an interface which doesn't make a
>> lot of sense.
>
> Concrete doesn't mean "instantiatable by the user".  It means
> "instantiatable period".

Interfaces are not supposed to be instantiatable by anyone.  The fact that 
object_new() is used to create the interface is an internal implementation detail.

Interfaces are stateless and by definition, never have an implementation on 
their own.  So object_new() of an interface type directly results in a useless 
object.

Regards,

Anthony Liguori

>
> Paolo
>
Paolo Bonzini June 13, 2012, 1:38 p.m. UTC | #3 
Il 13/06/2012 15:36, Anthony Liguori ha scritto:
>>>
>>
>> Concrete doesn't mean "instantiatable by the user".  It means
>> "instantiatable period".
> 
> Interfaces are not supposed to be instantiatable by anyone.  The fact
> that object_new() is used to create the interface is an internal
> implementation detail.

Whose side effect is that interfaces need to be concrete.

> Interfaces are stateless and by definition, never have an implementation
> on their own.  So object_new() of an interface type directly results in
> a useless object.

Yes, and that's why you needn't really care about what the user does.
Garbage in, garbage out...

Paolo
Anthony Liguori June 13, 2012, 1:50 p.m. UTC | #4 
On 06/13/2012 08:38 AM, Paolo Bonzini wrote:
> Il 13/06/2012 15:36, Anthony Liguori ha scritto:
>>>>
>>>
>>> Concrete doesn't mean "instantiatable by the user".  It means
>>> "instantiatable period".
>>
>> Interfaces are not supposed to be instantiatable by anyone.  The fact
>> that object_new() is used to create the interface is an internal
>> implementation detail.
>
> Whose side effect is that interfaces need to be concrete.
>
>> Interfaces are stateless and by definition, never have an implementation
>> on their own.  So object_new() of an interface type directly results in
>> a useless object.
>
> Yes, and that's why you needn't really care about what the user does.
> Garbage in, garbage out...

Quick chat on IRC and I think we both agree that we need to promote the notion 
of Interface to a first class concept in QOM.

I'll send out a patch later today (that's actually tested..).

Regards,

Anthony Liguori

>
> Paolo
>
Edgar E. Iglesias June 13, 2012, 8:22 p.m. UTC | #5 
this is not super on topic and might be a dumb question but now that we use
glib, why aren't we using gobjects more than we are? I'm guessing the glib
ppl have figured out how all this is suposed to work already...
On Jun 13, 2012 3:50 PM, "Anthony Liguori" <anthony@codemonkey.ws> wrote:

> On 06/13/2012 08:38 AM, Paolo Bonzini wrote:
>
>> Il 13/06/2012 15:36, Anthony Liguori ha scritto:
>>
>>>
>>>>>
>>>> Concrete doesn't mean "instantiatable by the user".  It means
>>>> "instantiatable period".
>>>>
>>>
>>> Interfaces are not supposed to be instantiatable by anyone.  The fact
>>> that object_new() is used to create the interface is an internal
>>> implementation detail.
>>>
>>
>> Whose side effect is that interfaces need to be concrete.
>>
>>  Interfaces are stateless and by definition, never have an implementation
>>> on their own.  So object_new() of an interface type directly results in
>>> a useless object.
>>>
>>
>> Yes, and that's why you needn't really care about what the user does.
>> Garbage in, garbage out...
>>
>
> Quick chat on IRC and I think we both agree that we need to promote the
> notion of Interface to a first class concept in QOM.
>
> I'll send out a patch later today (that's actually tested..).
>
> Regards,
>
> Anthony Liguori
>
>
>> Paolo
>>
>>
>
diff mbox

Patch

From c7e0661bd261ac4919dca0e3a5dd78d3871c3292 Mon Sep 17 00:00:00 2001
From: Anthony Liguori <aliguori@us.ibm.com>
Date: Wed, 13 Jun 2012 08:29:30 -0500
Subject: [PATCH] qom: allow interfaces to be created while abstract during
 object initialization

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qom/object.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 6f839ad..1a436d8 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -73,6 +73,8 @@  typedef struct Interface
 
 static Type type_interface;
 
+static Object *object_new_with_type_full(Type type, bool concrete_only);
+
 static GHashTable *type_table_get(void)
 {
     static GHashTable *type_table;
@@ -250,7 +252,7 @@  static void object_interface_init(Object *obj, InterfaceImpl *iface)
     TypeImpl *ti = iface->type;
     Interface *iface_obj;
 
-    iface_obj = INTERFACE(object_new(ti->name));
+    iface_obj = INTERFACE(object_new_with_type_full(ti, false));
     iface_obj->obj = obj;
 
     obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj);
@@ -273,7 +275,8 @@  static void object_init_with_type(Object *obj, TypeImpl *ti)
     }
 }
 
-void object_initialize_with_type(void *data, TypeImpl *type)
+static void object_initialize_with_type_full(void *data, TypeImpl *type,
+                                             bool concrete_only)
 {
     Object *obj = data;
 
@@ -281,7 +284,7 @@  void object_initialize_with_type(void *data, TypeImpl *type)
     type_initialize(type);
 
     g_assert(type->instance_size >= sizeof(Object));
-    g_assert(type->abstract == false);
+    g_assert(!concrete_only || type->abstract == false);
 
     memset(obj, 0, type->instance_size);
     obj->class = type->class;
@@ -289,6 +292,11 @@  void object_initialize_with_type(void *data, TypeImpl *type)
     object_init_with_type(obj, type);
 }
 
+void object_initialize_with_type(void *data, TypeImpl *type)
+{
+    object_initialize_with_type_full(data, type, false);
+}
+
 void object_initialize(void *data, const char *typename)
 {
     TypeImpl *type = type_get_by_name(typename);
@@ -362,7 +370,7 @@  void object_finalize(void *data)
     g_assert(obj->ref == 0);
 }
 
-Object *object_new_with_type(Type type)
+static Object *object_new_with_type_full(Type type, bool concrete_only)
 {
     Object *obj;
 
@@ -370,12 +378,17 @@  Object *object_new_with_type(Type type)
     type_initialize(type);
 
     obj = g_malloc(type->instance_size);
-    object_initialize_with_type(obj, type);
+    object_initialize_with_type_full(obj, type, concrete_only);
     object_ref(obj);
 
     return obj;
 }
 
+Object *object_new_with_type(Type type)
+{
+    return object_new_with_type_full(type, true);
+}
+
 Object *object_new(const char *typename)
 {
     TypeImpl *ti = type_get_by_name(typename);
-- 
1.7.5.4