On 01/13/2012 10:58 AM, Daniel P. Berrange wrote:
On Thu, Jan 12, 2012 at 11:59:22AM +0000, Matthew Booth wrote:
> I'm currently working on gobject bindings for libguestfs. I haven't
> got as far as compiling anything yet, but I've attached the C header
> for initial review.
> typedef struct _GuestfsPV GuestfsPV;
> struct _GuestfsPV {
> gchar *pv_name;
> /* The next field is NOT nul-terminated, be careful when printing it: */
> gchar pv_uuid[32];
How about a:
#define GUESTFS_LVM_UUID_BUFLEN 32
and use that throughout
I don't think this one is massively important, because you can always
use sizeof pv_uuid as an equivalent constant. As the API is
automatically generated, these are always going to be consistent.
I'd also make this change in the C header and use it here. Rich?
Obviously all these methods should have 'guestfs_session_'
as a prefix
Whoops :)
> /* Generated methods */
> gboolean test0(GuestfsSession *session, const gchar *str, const gchar *optstr, GSList
*strlist, gboolean b, gint32 integer, gint64 integer64, const gchar *filein, const gchar
*fileout, GByteArray *bufferin, GError **err);
> gint32 test0rint(GuestfsSession *session, const gchar *val, GError **err);
> gint32 test0rinterr(GuestfsSession *session, , GError **err);
> gint64 test0rint64(GuestfsSession *session, const gchar *val, GError **err);
> gint64 test0rint64err(GuestfsSession *session, , GError **err);
> gboolean test0rbool(GuestfsSession *session, const gchar *val, GError **err);
> gboolean test0rboolerr(GuestfsSession *session, , GError **err);
> gchar *test0rconststring(GuestfsSession *session, const gchar *val, GError **err);
> gchar *test0rconststringerr(GuestfsSession *session, , GError **err);
> gchar *test0rconstoptstring(GuestfsSession *session, const gchar *val);
> gint32 wc_c(GuestfsSession *session, const gchar *path, GError
**err);
> GSList *head(GuestfsSession *session, const gchar *path, GError **err);
> GSList *head_n(GuestfsSession *session, gint32 nrlines, const gchar *path, GError
**err);
> GSList *tail(GuestfsSession *session, const gchar *path, GError **err);
> GSList *tail_n(GuestfsSession *session, gint32 nrlines, const gchar *path, GError
**err);
> gchar *df(GuestfsSession *session, , GError **err);
> gchar *df_h(GuestfsSession *session, , GError **err);
Some missing parameters here& in a few other cases.
Fixed, thanks.
For the vast majority of these, or at least, any which take
non-trivial
execution time, I think it would be important to add _async variants,
following the GIO design pattern. eg
void guest_session_tune2fs_async(GuestfsSession *session, const gchar *device,
GuestfsTune2FS *optargs,
GCancellable *cancellable,
GAsyncReadyCallback callback,
gpointer user_data);
gboolean guest_session_tune2fs_finish(GAsyncResult *result,
GError **error);
This is a style that is widely preferred by people writing modern GTK
applications, since it avoids the need to use threads in GUI apps.
Sounds like a good idea. Can I assume that I can safely add this at a
later date without taking it into account at this stage? If so, I'll
come back to this once I have a working binding.
Incidentally, I'm also playing with changes to functions with return or
take array. I've decided to drop GSList in favour of a NULL-terminated
array as returned by guestfs. For BufferIn and RBufferOut I've dropped
the GByteArray in favour of a guint8 * with an additional length parameter.
My concern is around memory handling in the bindings. An RStringList
function returns a NULL-terminated char **. If I cast that to gchar **
and return it as-is, will bindings correctly garbage collect it?
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