Re: Mango comments ...

[Mathieu Lacage <mathieu eazel com>]

Michael Meeks <michael helixcode com> writes:

> Hi Guys,
> 
> 	Hi read most of it with my brain turned on, towards the end it
> switched off a bit. I'm not very convinced :-) I think we just need to
> learn to love the OMG binding, but I'll watch with interest as you
> progress.

we will see :)

> 
> 	Here's the patch:


Thanks a lot.

> 
> ? a.txt
> Index: doc/design-goal.sgml
> ===================================================================
> RCS file: /cvs/gnome/mango/doc/design-goal.sgml,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 design-goal.sgml
> --- doc/design-goal.sgml	2000/11/07 04:43:30	1.1.1.1
> +++ doc/design-goal.sgml	2000/11/10 22:46:21
> @@ -35,8 +35,8 @@
>        <para>
>        &mango; is a CORBA mapping for C using the <ulink
>        url="http://www.gtk.org";>Glib</ulink> library. Glib was
> -      developed for the Gtk+ toolkit as an abstraction layer nad has
> -      evolved to a general purpose utility library.  &mango; stands
> +      developed for the Gtk+ toolkit as an abstraction layer and has
> +      evolved into a general purpose utility library.  &mango; stands
>        for the silly recursive acronym: <emphasis>Mango Allows Nice
>        Gnome Objects</emphasis>.
>        </para>
> @@ -52,11 +52,12 @@
>        in C. It features a completely dynamic type system, a signaling
>        system and single inheritance.  GObject is the base Object for
>        all Gtk+ and Gnome widgets and Gtk+ programmers are used
> -      to its syntax and semantics.
> +      to its syntax and semantics. GObject also allows Java like
> +      interfaces.
>        </para>
>  
>        <para>
> -      The OMG-specified C mapping has a number of shortcomminsg but
> +      The OMG-specified C mapping has a number of shortcommings but
>        the main one is that its use is clumsy in Gtk+
>        programs. Currently, a number of Gnome core libraries have
>        developed Gtk+ Wrapper Objects for CORBA objects. Those Gtk+
> @@ -69,7 +70,7 @@
>        </para>
>  
>        <para>
> -      The above approach has a number of shortcommings. For example,
> +      The above approach has a number of shortcomings. For example,
>        the real CORBA objects are not completely hidden 
>        behind the GTK+ proxies making the life of users a real nightmare 
>        (you have to deal with numerous memory management policies and
> @@ -79,7 +80,7 @@
>        </para>
>  
>        <para>
> -      &mango;'s main goal to avoid having to write all those
> +      &mango;'s main goal is to avoid having to write all those
>        cumbersome Gtk+ wrappers by hand and to simplify the use of the
>        IDL interfaces designed in Bonobo. We want people to be able to
>        use the IDL directly, through a mapping better integrated with
> @@ -186,7 +187,7 @@
>  		  <row>
>  		    <entry>octet</entry>
>  		    <entry>8 bit byte</entry>
> -		    <entry>char (or should it be guint8?)</entry>
> +		    <entry>guint8</entry>
>  		  </row>
>  		  
>  		  <row>
> @@ -293,7 +294,8 @@
>        <note>
>        <para>
>        We could support it to be compliant by using 
> -      <function>gtk_object_{set|get}_data_full</function>... Not really important for now.
> +      <function>gtk_object_{set|get}_data_full</function>...
> +      Not really important for now. ? Is this intended to be here in the text?

nope.

>        </para>      
>        </note>      
>        </para>
> @@ -388,7 +390,7 @@
>        };
>  };
>  </programlisting>
> -      foo::bar::foo would map to <function>Mfoo_bar_foo (foo_bar *object, GError *error)</function>.
> +      foo::bar::foo would map to <function>Mfoo_bar_foo (foo_bar *object, GError **error)</function>.
>        <emphasis>::</emphasis> are mapped to <emphasis>_</emphasis> and a 
>        <emphasis>M</emphasis> is prepended to avoid clashing with the standard
>        C mapping functions. (it is very likely applications will link with some
> @@ -472,7 +474,7 @@
>      <para>
>      The problem which you might begin to understand with the above scenario is that the activation system
>      will return a <classname>CORBA::Object</classname> (which in our case is effectively a 
> -    <classname>MCORBA_Object</classname>). What you want to do then is downcast this object to
> +    <classname>MCORBA_Object</classname>). What you want to do then is narrow this object to
>      <classname>MBonobo_Unknown</classname> and <classname>MBonobo_Control</classname>. The thing is that
>      while in the OMG C mapping they are the exact same structure, they are not the same in this proposed
>      mapping.
> @@ -501,13 +503,17 @@
>      </para>
>      
>      <para>
> -    Determining if downcasting is right can be done as folows:
> +    Validating the narrowing can be done as folows:
>      When the casting macro is called, you do 2 checks.
>      <itemizedlist>
>      <listitem><para>
>      First, you take the repoid of the current object, you take the repo id of the object you want to 
>      cast to and you test through <function>CORBA::Object::is_a</function>. Since ORBit does not implement
> -    this function, we need to implement it. A possible way would be to make each skeleton when it is mapped 
> +    this function, 
> +
> +    ??? See ORBit/src/orb/corba_object.[ch] (CORBA_Object_is_a) ???


yes, stupid me.

> +
> +    we need to implement it. A possible way would be to make each skeleton when it is mapped 
>      in memory register the types of the objects it implements and the inheritance tree between these 
>      objects. (this would involve some hacking to get some code in the .init segment of the generated binary
>      but I am confident we can do this through the use of a linker script.) This sounds like a job for the 
> @@ -542,6 +548,10 @@
>      first place. MICO takes a different tack - the _narrow operation
>      generated in the stubs actually constructs a fresh proxy class
>      from the superclass reference when downcasting.
> +
> +    ? downcasting; it sounds like someone is getting upset :-), is this
> +      standard terminology ?

I know the C++ mapping uses narrow as a name for this concept but it really is
a downcast and I think that this terminology is used only for the C++ mapping.

Am I wrong ?

> +
>      </para>
>  
>  
> @@ -568,6 +578,10 @@
>        parent object in the object structure 3) use this ofset to add it to the object pointer...
>        </para>
>  
> +      ? This confuses me; it seems you are advocating flattening the aggregate into what
> +      looks like a multiply-inherited object; by perhaps you are talking about the MI on
> +      the interfaces in the aggregate ?

nah, just trying to figure out how to make the above scheme work in multiple-inherited objects.

> +
>        <para>
>        Multiple inheritance support is not a priority though so... We will see later.
>        </para>
> @@ -586,8 +600,8 @@
>  </programlisting>
>        The above would map to
>  <programlisting>
> -gint32    Mfoo__get_size (foo reference, GError *error);
> -void      Mfoo__set_size (foo reference, gint32 size, GError *error);
> +gint32    Mfoo__get_size (foo reference, GError **error);
> +void      Mfoo__set_size (foo reference, gint32 size, GError **error);
>  </programlisting>
>        Each attribute is basically mapped to the folowing illegal IDL:
>  <programlisting>
> @@ -622,6 +636,9 @@
>        mechanism).
>        </para>
>  
> +      ? GParam will do automatic type coercion for you though, which
> +        is a nice feature for users ?
> +
>        </sect2>
>  
>        <sect2>
> @@ -644,13 +661,13 @@
>        it means something else. Also, inouts make memory management for
>        the user confusing. So, they suck, let's get rid of them. (one
>        should note that at the IIOP level, an inout and an in and an
> -      out and completely equivalent) On the other hand, mapping inout
> +      out are completely equivalent) On the other hand, mapping inout
>        might not be hard and could be done as a future extension.
>        </para>
>  
>        <para>
> -      Now, it is simple: <emphasis>out</emphasis> parameters are always passed a pointer to 
> -      the actual variable where the data is to be returned. The returned data shold be freed
> +      Now, it is simple: <emphasis>out</emphasis> parameters are always passed as a pointer to 
> +      the actual variable in which the data is to be returned. The returned data shold be freed
>        with <function>g_free</function> or the appropriate freeing function for the concerned 
>        type.
>        <emphasis>in</emphasis> parameters will follow the same rule as that of the C mapping:
> @@ -823,10 +840,32 @@
>        use directly the native GObject mechanism to handle the memory management of the Object 
>        references: <function>CORBA::Object::duplicate</function> is thus a direct call to 
>        <function>g_object_ref</function> and <function>CORBA::Object::release</function> is 
> -      a direct call to <function>g_object_ref</function>. Users are actually encouraged to
> +      a direct call to <function>g_object_unref</function>. Users are actually encouraged to
>        use directly <function>g_object_{ref|unref}</function>.
>        </para>
>  
> +? Have you seen this:
> +
> +	Despite my requests; Tim has not made ref / unref virtual,
> +nor even added the hooks I suggested; hey ho.
> +
> +GObject*
> +g_object_ref (GObject *object)
> +{
> +  g_return_val_if_fail (G_IS_OBJECT (object), NULL);
> +  g_return_val_if_fail (object->ref_count > 0, NULL);
> +  
> +  object->ref_count += 1;
> +  
> +  return object;
> +}
> +
> +	Either way, it actualy doesn't matter at all; as long as you keep
> +a single CORBA / Bonobo reference on the GObject and use g_object_ref / unref
> +localy there is no problem.
> +
> +?
> +
>        <para>
>        The rest of the CORBA::Object interface should be used as expected given the method 
>        signatures. (Anyone disagrees on this ?)
> @@ -885,7 +924,10 @@
>    GObjectClass parent_class;
>    
>    /* virtual method */
> -  gboolean (*MPOA_Foo_bar) (MFoo_bar *self, gint32 baz, GError *error);
> +  gboolean (*MPOA_Foo_bar) (MFoo_bar *self, gint32 baz, GError **error);
> +
> +  ? Why mangle the full name here, not even the OMG do that ?
> +
>  };


hrm, really ?

I did not knew: you are right it is not really necessary but it might make it easier
to make the server-side servant and the client-side proxy look the same.

>  
>  </programlisting>
> @@ -898,6 +940,9 @@
>        <function>destroy</function> virtual methods can do the same...
>        Also, we will need an <function>activate</function> virtual method probably. (really need 
>        to think about it)
> +
> +      ? What does the activate method do ?

If i remmember well, I think it is a factory method used by the POA to instantiate
the relevant servant when given certain POA::Policies....

> +
>        </para>
>  
>        <para>
> @@ -949,7 +994,7 @@
>        <title>Client side</title>
>  
>        <para>
> -      The stubs define the <classname>Mfoo_bar</classname> GObjbect and the 
> +      The stubs define the <classname>Mfoo_bar</classname> GObject and the 
>        <function>Mfoo_bar_baz</function> function.
>        We imagine OAF was converted to the new mapping.
>  <programlisting>
> @@ -957,10 +1002,13 @@
>  {
>        MCORBA_Object *object_ref;
>        GError *error;
> +
> +      object_ref = oaf_activate ("has (repo_ids, 'IDL:Bonobo::Control 1.0')", NULL, 0, NULL, NULL)
>  
> -      object_ref = oaf_activate ("has (repo_ids, "IDL:Bonobo::Control 1.0")", NULL, 0, NULL, NULL)
> +      ? I assume you specify the toleration of a NULL exception environment, a
> +        worthwhile enough feature I think ?

hrm, no: this function was simply a call to the OAF API.

The OAF API is NULL tolerant as much as possible.

>  
> -      Mfoo_bar_baz (Mfoo_bar (object_ref), 0, error);
> +      Mfoo_bar_baz (Mfoo_bar (object_ref), 0, &error);
>        
>        return 0;
>  }
> @@ -989,7 +1037,7 @@
>    GObjectClass parent_class;
>    
>    /* virtual method */
> -  void (*MPOA_foo_bar_baz) (MPOA_foo_bar *self, gint32 value, GError *error);
> +  void (*MPOA_foo_bar_baz) (MPOA_foo_bar *self, gint32 value, GError **error);
>  };
>  </programlisting>
>        To implement it, you would create the folowing .h file (the generation of this file
> @@ -1070,8 +1118,8 @@
>  /* then, our implementation of the servant's method: */
>  static
>  void MPOA_foo_bar_baz (MPOA_foo_bar *self, 
> -                       gint32 value, 
> -                       GError *error)
> +                       gint32        value, 
> +                       GError      **error)
>  {
>         self->value = value;
>  }


Thanks for this feedback,
Mathieu

-- 
Mathieu Lacage <mathieu eazel com>