I've summarised how the GObject bindings work below, which should hopefully help reviewing the generator/generated code. There are a couple of points in here I'm still not 100% happy with. Specifically the handling of FBuffer and the Cancellable flag. Both are explained below. I'm interested in suggestions. Return values: ************** All functions which can return an error have as their final argument a GError **. GI maps this to the appropriate error mechanism for each target language. RErr returns a gboolean, true = success, false = failure. The following types return the value from libguestfs unchanged: RInt RInt64 RBool RConstString (transfer none): gobject doesn't manage returned memory RConstOptString (transfer none). Methods don't return an error (no GError**) RString RStringList RHashtable is converted to a GHashTable. The keys and values from libguestfs are inserted directly into the GHashTable without copying. The top level array returned by libguestfs is freed. RStruct is converted to GObject boxed type for the target struct. It returns a copy of the struct, which is copied field by field. Memory for the returned struct is allocated using glib's slice allocator. The original struct is freed with guestfs_free_<struct>(). RStructList is converted as an array of structs, which are individually converted the same way as RStruct, i.e. everything is copied. The returned array is a newly allocated NULL-terminated array. The originally returned value is freed with guestfs_free_<struct>_list(). RBufferOut is returned as a guint8 array, whose length is specified by a size_r field. GObject uses both these fields to construct a single array object where language bindings allow. The value returned by RBufferOut is not copied. Structs: ******** A parallel struct is created for each libguestfs struct type. A boxed type is created for each parallel struct. It's field types are all mapped trivially to gobject basic types (e.g. gchar *, gint32) except FBuffer. FBuffer is mapped as: guint8 *<field> guint32 <field>_size Unfortunately I can see no way to attach annotation to these fields, so the bindings do not appreciate that these fields are related. I'm not happy with this at all, so I may try manual tweaking of the .gir to see if we can turn these into a single returned array where possible. Required arguments: ******************* Required arguments to methods are all mapped trivially to gobject basic types. They are all passed directly to libguestfs. StringList and DeviceList both remain null-terminated arrays. BufferIn has separate array and length components, which are combined into a single array using annotations where language bindings permit. Optional arguments: ******************* Optional arguments are implemented as a separate gobject whose name is is Guestfs<camel api name>. This gobject has defined properties for each optional argument accepted by the api. All properties are initially undefined. Property types are mapped as: OBool -> GuestfsTristate OInt, OInt64 -> gint/gint64. -1 is used internally as a magic value to represent undefined. OString -> gchar *. NULL represents undefined. GuestfsTristate is a new boxed enum type with values FALSE, TRUE and UNSET. In the javascript bindings at least, passing in 'true' or 'false' here works as expected. To set the UNSET value, you have to explicitly use GuestfsTristate.UNSET. OInt and OInt64 have a similar problem to boolean, in that there's no way to represent unset in the fundamental type. The -1 = undefined scheme is an internal detail and need not be part of the API. I did create a patch to make the undefined number explicit for each optional argument which required it. However I decided to hold off on it until we actually create an api where this would be a problem, as it makes the api definition in the generator slightly uglier and more complex. Where no optional arguments are required, the user can pass in the binding language's equivalent of NULL. GObject has a placeholder for default values, but there are not yet implemented. When they are, we can define the default value of optargs to be null, meaning they can be entirely omitted when not required. Cancellation: ************* Certain apis are cancellable. These all take a GCancellable as the final argument before GError **. This can be passed NULL if cancellation is not required. While I have written cancellation, I have not yet tested it *at all* other than it compiles and works correctly when NULL is passed in. We recently made Cancellable an explicit flag whereas before it was implicit if the api had a FileIn or FileOut argument. This means it is now possible to break the GObject api without breaking the C api with the addition of a Cancellable flag. What potential solutions are there to this problem? I can see: ? Live with breaking the GObject api if it ever comes up. ? Never add Cancellable to an existing api. ? Automatically add a GCancellable argument to all GObject apis, just in case. Events API ********** The libguestfs events API is not yet bound. I'll add this at a later stage. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
On Fri, Jan 20, 2012 at 12:07:47PM +0000, Matthew Booth wrote:> I've summarised how the GObject bindings work below, which should > hopefully help reviewing the generator/generated code. > > There are a couple of points in here I'm still not 100% happy with. > Specifically the handling of FBuffer and the Cancellable flag. Both > are explained below. I'm interested in suggestions. > > Return values: > ************** > > All functions which can return an error have as their final argument > a GError **. GI maps this to the appropriate error mechanism for > each target language. > > RErr returns a gboolean, true = success, false = failure. > The following types return the value from libguestfs unchanged: > RInt > RInt64 > RBool > RConstString (transfer none): gobject doesn't manage returned memory > RConstOptString (transfer none). Methods don't return an error (no > GError**) > RString > RStringList > > RHashtable is converted to a GHashTable. The keys and values from > libguestfs are inserted directly into the GHashTable without > copying. The top level array returned by libguestfs is freed.It could be useful to run the gobject tests under valgrind. See tests/extra/Makefile.am> RStruct is converted to GObject boxed type for the target struct. It > returns a copy of the struct, which is copied field by field. Memory > for the returned struct is allocated using glib's slice allocator. > The original struct is freed with guestfs_free_<struct>(). > > RStructList is converted as an array of structs, which are > individually converted the same way as RStruct, i.e. everything is > copied. The returned array is a newly allocated NULL-terminated > array. The originally returned value is freed with > guestfs_free_<struct>_list(). > > RBufferOut is returned as a guint8 array, whose length is specified > by a size_r field. GObject uses both these fields to construct a > single array object where language bindings allow. The value > returned by RBufferOut is not copied. > > Structs: > ******** > > A parallel struct is created for each libguestfs struct type. A > boxed type is created for each parallel struct. It's field types are > all mapped trivially to gobject basic types (e.g. gchar *, gint32) > except FBuffer. FBuffer is mapped as: > > guint8 *<field> > guint32 <field>_size > > Unfortunately I can see no way to attach annotation to these fields, > so the bindings do not appreciate that these fields are related. I'm > not happy with this at all, so I may try manual tweaking of the .gir > to see if we can turn these into a single returned array where > possible. > > Required arguments: > ******************* > > Required arguments to methods are all mapped trivially to gobject > basic types. They are all passed directly to libguestfs. StringList > and DeviceList both remain null-terminated arrays. BufferIn has > separate array and length components, which are combined into a > single array using annotations where language bindings permit. > > Optional arguments: > ******************* > > Optional arguments are implemented as a separate gobject whose name > is is Guestfs<camel api name>. This gobject has defined properties > for each optional argument accepted by the api. All properties are > initially undefined. Property types are mapped as: > > OBool -> GuestfsTristate > OInt, OInt64 -> gint/gint64. -1 is used internally as a magic > value to represent undefined. > OString -> gchar *. NULL represents undefined. > > GuestfsTristate is a new boxed enum type with values FALSE, TRUE and > UNSET. In the javascript bindings at least, passing in 'true' or > 'false' here works as expected. To set the UNSET value, you have to > explicitly use GuestfsTristate.UNSET. > > OInt and OInt64 have a similar problem to boolean, in that there's > no way to represent unset in the fundamental type. The -1 > undefined scheme is an internal detail and need not be part of the > API. I did create a patch to make the undefined number explicit for > each optional argument which required it. However I decided to hold > off on it until we actually create an api where this would be a > problem, as it makes the api definition in the generator slightly > uglier and more complex. > > Where no optional arguments are required, the user can pass in the > binding language's equivalent of NULL. GObject has a placeholder for > default values, but there are not yet implemented. When they are, we > can define the default value of optargs to be null, meaning they can > be entirely omitted when not required. > > Cancellation: > ************* > > Certain apis are cancellable. These all take a GCancellable as the > final argument before GError **. This can be passed NULL if > cancellation is not required. While I have written cancellation, I > have not yet tested it *at all* other than it compiles and works > correctly when NULL is passed in. > > We recently made Cancellable an explicit flag whereas before it was > implicit if the api had a FileIn or FileOut argument. This means it > is now possible to break the GObject api without breaking the C api > with the addition of a Cancellable flag. What potential solutions > are there to this problem? I can see: > > ? Live with breaking the GObject api if it ever comes up. > ? Never add Cancellable to an existing api. > ? Automatically add a GCancellable argument to all GObject apis, > just in case.The right way is to add a check to generator_checks.ml which ensures that presence of FileIn/FileOut === Cancellable flag in flags.> Events API > ********** > > The libguestfs events API is not yet bound. I'll add this at a later stage. > > MattRich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Fri, Jan 20, 2012 at 12:07:47PM +0000, Matthew Booth wrote:> There are a couple of points in here I'm still not 100% happy with. > Specifically the handling of FBuffer and the Cancellable flag. Both > are explained below. I'm interested in suggestions. > A parallel struct is created for each libguestfs struct type. A > boxed type is created for each parallel struct. It's field types are > all mapped trivially to gobject basic types (e.g. gchar *, gint32) > except FBuffer. FBuffer is mapped as: > > guint8 *<field> > guint32 <field>_size > > Unfortunately I can see no way to attach annotation to these fields, > so the bindings do not appreciate that these fields are related. I'm > not happy with this at all, so I may try manual tweaking of the .gir > to see if we can turn these into a single returned array where > possible.Those two fields look just like GByteArray struct GByteArray { guint8 *data; guint len; }; Can you just outpout that - I expect bindings will already know how to deal with that.> Cancellation: > ************* > > Certain apis are cancellable. These all take a GCancellable as the > final argument before GError **. This can be passed NULL if > cancellation is not required. While I have written cancellation, I > have not yet tested it *at all* other than it compiles and works > correctly when NULL is passed in. > > We recently made Cancellable an explicit flag whereas before it was > implicit if the api had a FileIn or FileOut argument. This means it > is now possible to break the GObject api without breaking the C api > with the addition of a Cancellable flag. What potential solutions > are there to this problem? I can see: > > ? Live with breaking the GObject api if it ever comes up. > ? Never add Cancellable to an existing api. > ? Automatically add a GCancellable argument to all GObject apis, > just in case.I think option 2 is the only long term viable approach. If any existing APIs needs to be made cancellable, then define a new API that is the same, but with the cancellable flag set. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|