[Libosinfo] [libosinfo PATCH 2/6] Introduce OsinfoImage object

[Richard W.M. Jones]

On Wed, Oct 31, 2018 at 10:45:34PM +0100, Fabiano Fidêncio wrote:
> +static gboolean osinfo_image_get_guestfs_info(const gchar *location,
> +                                              gchar **arch,
> +                                              gchar **osinfo,
> +                                              gchar **product_name)
> +{
> +    gboolean ret = FALSE;
> +
> +#ifdef HAVE_GUESTFS
> +    gchar **roots = NULL;
> +    guestfs_h *guest;

We'd normally call the handle just 'g' :-)

> +    guest = guestfs_create();
> +    if (guest == NULL)
> +        return FALSE;
> +
> +    if (guestfs_add_drive_opts(guest, location,
> +                               GUESTFS_ADD_DRIVE_OPTS_READONLY, 1,
> +                               -1) == -1)
> +        goto done;
> +
> +    if (guestfs_launch(guest) == -1)
> +        goto done;

Do you care where error messages go to?  guestfs will send the error
messages to stderr by default, although of course it's possible to
override this.  You can set or push an error handler callback
function.  This is all explained here:

http://libguestfs.org/guestfs.3.html#error-handling

> +    roots = guestfs_inspect_os(guest);
> +    if (roots == NULL || roots[0] == NULL)
> +        goto done;
> +
> +    *arch = guestfs_inspect_get_arch(guest, roots[0]);
> +    *osinfo = guestfs_inspect_get_osinfo(guest, roots[0]);
> +    *product_name = guestfs_inspect_get_product_name(guest, roots[0]);

I don't know if you care about error checking for these, but it's
possible (although very unlikely) for any of these three calls to
return NULL.

Also if inspection was successful but any of these results is not
known (possible, but somewhat unlikely), they will return the string
"unknown", and maybe you care about that case and want to do something
special.

> +    ret = TRUE;
> +
> + done:
> +    if (roots != NULL) {
> +        for (int i = 0; roots[i] != NULL; i++)
> +            g_free(roots[i]);
> +    }
> +    g_free(roots);
> +    guestfs_close(guest);
> +#endif /* HAVE_GUESTFS */

In general this all seems fine to me.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org