[virt-tools-list] [libosinfo v3] API to query required user avatar format

Zeeshan Ali (Khattak) zeeshanak at gnome.org
Thu Nov 22 13:51:59 UTC 2012


On Thu, Nov 22, 2012 at 11:26 AM, Christophe Fergeau
<cfergeau at redhat.com> wrote:
> On Wed, Nov 21, 2012 at 07:39:05PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak at gnome.org>
>>
>> Add a new API for apps to be able to query in which format do the user
>> avatar file need to be in.
>>
>> Based on a patch from Fabiano Fidêncio <fabiano at fidencio.org>.
>> ---
>>  data/install-scripts/fedora.xml      |   3 +
>>  data/install-scripts/windows-cmd.xml |   6 +
>>  data/schemas/libosinfo.rng           |  30 +++++
>>  osinfo/Makefile.am                   |   2 +
>>  osinfo/libosinfo.syms                |   8 +-
>>  osinfo/osinfo.h                      |   1 +
>>  osinfo/osinfo_avatar_format.c        | 242 +++++++++++++++++++++++++++++++++++
>>  osinfo/osinfo_avatar_format.h        |  95 ++++++++++++++
>>  osinfo/osinfo_install_script.c       |  51 ++++++++
>>  osinfo/osinfo_install_script.h       |   6 +
>>  osinfo/osinfo_loader.c               |  44 +++++++
>>  po/POTFILES.in                       |   1 +
>>  12 files changed, 488 insertions(+), 1 deletion(-)
>>  create mode 100644 osinfo/osinfo_avatar_format.c
>>  create mode 100644 osinfo/osinfo_avatar_format.h
>>
>> diff --git a/data/install-scripts/fedora.xml b/data/install-scripts/fedora.xml
>> index dc767d5..fcb1143 100644
>> --- a/data/install-scripts/fedora.xml
>> +++ b/data/install-scripts/fedora.xml
>> @@ -122,6 +122,9 @@ reboot
>>          <param name="avatar-disk" policy="optional"/>
>>          <param name="target-disk" policy="optional"/>
>>      </config>
>> +    <avatar-format>
>> +      <mime-type>image/png</mime-type>
>> +    </avatar-format>
>>      <template>
>>        <xsl:stylesheet
>>          xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
>> diff --git a/data/install-scripts/windows-cmd.xml b/data/install-scripts/windows-cmd.xml
>> index 020a493..84833a9 100644
>> --- a/data/install-scripts/windows-cmd.xml
>> +++ b/data/install-scripts/windows-cmd.xml
>> @@ -12,6 +12,12 @@
>>        <param name="target-disk" policy="optional"/>
>>        <param name="script-disk" policy="optional"/>
>>      </config>
>> +    <avatar-format>
>> +      <mime-type>image/bmp</mime-type>
>> +      <width>48</width>
>> +      <height>48</height>
>> +      <alpha>false</alpha>
>> +    </avatar-format>
>>      <template>
>>        <xsl:stylesheet
>>          xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
>> diff --git a/data/schemas/libosinfo.rng b/data/schemas/libosinfo.rng
>> index 3c57c36..87635dd 100644
>> --- a/data/schemas/libosinfo.rng
>> +++ b/data/schemas/libosinfo.rng
>> @@ -478,6 +478,9 @@
>>            </element>
>>          </optional>
>>          <optional>
>> +          <ref name='avatar-format'/>
>> +        </optional>
>> +        <optional>
>>            <element name='config'>
>>              <oneOrMore>
>>                <element name='param'>
>> @@ -509,6 +512,33 @@
>>      </element>
>>    </define>
>>
>> +  <define name='avatar-format'>
>> +    <element name='avatar-format'>
>> +      <interleave>
>> +        <oneOrMore>
>> +          <element name="mime-type">
>> +            <text/>
>> +          </element>
>> +        </oneOrMore>
>> +        <optional>
>> +          <element name="width">
>> +            <ref name='num'/>
>> +          </element>
>> +        </optional>
>> +        <optional>
>> +          <element name="height">
>> +            <ref name='num'/>
>> +          </element>
>> +        </optional>
>> +        <optional>
>> +          <element name="alpha">
>> +            <ref name='bool'/>
>> +          </element>
>> +        </optional>
>> +      </interleave>
>> +    </element>
>> +  </define>
>> +
>>    <define name="customElement">
>>      <element>
>>        <anyName/>
>> diff --git a/osinfo/Makefile.am b/osinfo/Makefile.am
>> index abaa78c..e85adb7 100644
>> --- a/osinfo/Makefile.am
>> +++ b/osinfo/Makefile.am
>> @@ -55,6 +55,7 @@ libosinfo_1_0_includedir = $(includedir)/libosinfo-1.0/osinfo
>>
>>  libosinfo_1_0_include_HEADERS = \
>>    osinfo.h                   \
>> +  osinfo_avatar_format.h     \
>>    osinfo_db.h                        \
>>    osinfo_loader.h            \
>>    osinfo_device.h            \
>> @@ -88,6 +89,7 @@ libosinfo_1_0_include_HEADERS = \
>>    $(NULL)
>>
>>  libosinfo_1_0_la_SOURCES =   \
>> +  osinfo_avatar_format.c     \
>>    osinfo_entity.c            \
>>    osinfo_enum_types.c                \
>>    osinfo_filter.c            \
>> diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
>> index de33a70..f3349d9 100644
>> --- a/osinfo/libosinfo.syms
>> +++ b/osinfo/libosinfo.syms
>> @@ -317,6 +317,12 @@ LIBOSINFO_0.2.1 {
>>
>>  LIBOSINFO_0.2.2 {
>>      global:
>> +     osinfo_avatar_format_get_type;
>> +     osinfo_avatar_format_get_mime_types;
>> +     osinfo_avatar_format_get_width;
>> +     osinfo_avatar_format_get_height;
>> +     osinfo_avatar_format_get_alpha;
>> +
>>       osinfo_install_config_param_policy_get_type;
>>       osinfo_media_error_get_type;
>>       osinfo_product_relationship_get_type;
>> @@ -336,10 +342,10 @@ LIBOSINFO_0.2.2 {
>>       osinfo_install_config_get_script_disk;
>>       osinfo_install_config_set_script_disk;
>>
>> +     osinfo_install_script_get_avatar_format;
>>       osinfo_install_script_get_path_format;
>>  } LIBOSINFO_0.2.1;
>>
>> -
>>  /* Symbols in next release...
>>
>>    LIBOSINFO_0.0.2 {
>> diff --git a/osinfo/osinfo.h b/osinfo/osinfo.h
>> index 81ed1cc..559eaa8 100644
>> --- a/osinfo/osinfo.h
>> +++ b/osinfo/osinfo.h
>> @@ -39,6 +39,7 @@
>>  #include <osinfo/osinfo_install_config.h>
>>  #include <osinfo/osinfo_install_config_param.h>
>>  #include <osinfo/osinfo_install_script.h>
>> +#include <osinfo/osinfo_avatar_format.h>
>>  #include <osinfo/osinfo_install_scriptlist.h>
>>  #include <osinfo/osinfo_productlist.h>
>>  #include <osinfo/osinfo_product.h>
>> diff --git a/osinfo/osinfo_avatar_format.c b/osinfo/osinfo_avatar_format.c
>> new file mode 100644
>> index 0000000..fb39da9
>> --- /dev/null
>> +++ b/osinfo/osinfo_avatar_format.c
>> @@ -0,0 +1,242 @@
>> +/*
>> + * libosinfo:
>> + *
>> + * Copyright (C) 2009-2012 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + *
>> + * Authors:
>> + *   Fabiano Fidêncio <fabiano at fidencio.org>
>> + *   Zeeshan Ali (Khattak) <zeeshanak at gnome.org>
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include <osinfo/osinfo.h>
>> +#include <glib/gi18n-lib.h>
>> +
>> +G_DEFINE_TYPE (OsinfoAvatarFormat, osinfo_avatar_format, OSINFO_TYPE_ENTITY);
>> +
>> +#define OSINFO_AVATAR_FORMAT_GET_PRIVATE(obj)  \
>> +        (G_TYPE_INSTANCE_GET_PRIVATE ((obj),                \
>> +         OSINFO_TYPE_AVATAR_FORMAT,            \
>> +         OsinfoAvatarFormatPrivate))
>> +
>> +/**
>> + * SECTION: osinfo_avatar_format
>> + * @short_description: The required format of avatar for an install script
>> + * @see_also: #OsinfoInstallScript
>> + */
>> +
>> +enum {
>> +    PROP_0,
>> +
>> +    PROP_MIME_TYPES,
>> +    PROP_WIDTH,
>> +    PROP_HEIGHT,
>> +    PROP_ALPHA,
>> +};
>> +
>> +static void
>> +osinfo_avatar_format_get_property(GObject *object,
>> +                               guint property_id,
>> +                                  GValue *value,
>> +                                  GParamSpec *pspec)
>> +{
>> +    OsinfoAvatarFormat *avatar = OSINFO_AVATAR_FORMAT (object);
>
> NB: there are several places with space before '(' in this patch, while
> most of it has no space, but libosinfo code base is highly inconsistent in
> that regard.

Yeah, AFAICT the gobject boilerplate code was copied over from
somewhere but not completely adapted to libosinfo's coding style.
Since then it has been copied over everywhere and I have been trying
to be consistent with the inconsistency. :)

>> +
>> +    switch (property_id)
>> +        {
>> +        case PROP_MIME_TYPES:
>> +        {
>> +            GList *mime_types;
>> +
>> +            mime_types = osinfo_avatar_format_get_mime_types(avatar);
>> +            g_value_set_pointer(value, mime_types);
>> +            break;
>> +        }
>
> Does that work nicely with gobject-introspection?

Yes and also with Vala.

> In particular, is pygtk
> able to handle it? I seem to remember some problems there.

I was told that this should work with python bindings at least and
when I reported back that it works with Vala bindings out of the box,
tomeu on #introspection was impressed. :)  If it doesn't actually work
for pygtk, its really not our problem as long as
gobject-instrospection (from Boxes POV, vapigen) supports it and can
handle it nicely.

>> +        case PROP_WIDTH:
>> +            g_value_set_int(value,
>> +                            osinfo_avatar_format_get_width(avatar));
>> +            break;
>> +        case PROP_HEIGHT:
>> +            g_value_set_int(value,
>> +                            osinfo_avatar_format_get_height(avatar));
>> +            break;
>> +        case PROP_ALPHA:
>> +            g_value_set_boolean(value,
>> +                                osinfo_avatar_format_get_alpha(avatar));
>> +            break;
>> +        default:
>> +            /* We don't have any other property... */
>> +            G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
>> +            break;
>> +        }
>> +}
>> +
>> +/* Init functions */
>> +static void
>> +osinfo_avatar_format_class_init (OsinfoAvatarFormatClass *klass)
>> +{
>> +    GObjectClass *g_klass = G_OBJECT_CLASS (klass);
>> +    GParamSpec *pspec;
>> +
>> +    g_klass->get_property = osinfo_avatar_format_get_property;
>> +
>> +    /**
>> +     * OsinfoAvatarFormat:mime-types:
>> +     *
>> +     * The allowed mime-types for the avatar.
>> +     *
>> +     * Type: GLib.List(utf8)
>> +     * Transfer: container
>> +     **/
>> +    pspec = g_param_spec_pointer("mime-types",
>> +                                "MIME Types",
>> +                                _("The allowed mime-types for the avatar"),
>> +                                G_PARAM_READABLE |
>> +                                G_PARAM_STATIC_STRINGS);
>> +    g_object_class_install_property(g_klass,
>> +                                    PROP_MIME_TYPES,
>> +                                    pspec);
>> +
>> +    /**
>> +     * OsinfoAvatarFormat:width:
>> +     *
>> +     * The required width (in pixels) of the avatar.
>> +     **/
>> +    pspec = g_param_spec_int("width",
>> +                             "Width",
>> +                             _("The required width (in pixels) of the avatar"),
>> +                             -1,
>> +                             G_MAXINT,
>> +                             -1,
>> +                             G_PARAM_READABLE |
>> +                             G_PARAM_STATIC_STRINGS);
>
> I think the default value will not get set without G_PARAM_CONSTRUCT_ONLY |
> G_PARAM_WRITABLE, did you test that specific case?

We don't need or want to set the default prop. We just translate the
string we get from XML to int on runtime and if string is not there,
we assume the default: -1.

>> +    g_object_class_install_property(g_klass,
>> +                                    PROP_WIDTH,
>> +                                    pspec);
>> +
>> +    /**
>> +     * OsinfoAvatarFormat:height:
>> +     *
>> +     * The required height (in pixels) of the avatar.
>> +     **/
>> +    pspec = g_param_spec_int("height",
>> +                             "Height",
>> +                             _("The required height (in pixels) of the avatar."),
>> +                             -1,
>> +                             G_MAXINT,
>> +                             -1,
>> +                             G_PARAM_READABLE |
>> +                             G_PARAM_STATIC_STRINGS);
>> +    g_object_class_install_property(g_klass,
>> +                                    PROP_HEIGHT,
>> +                                    pspec);
>> +
>> +    /**
>> +     * OsinfoAvatarFormat:alpha:
>> +     *
>> +     * Whether alpha channel is supported in the avatar.
>> +     **/
>> +    pspec = g_param_spec_boolean("alpha",
>> +                                 "Alpha",
>> +                                 _("Whether alpha channel is supported in the avatar."),
>> +                                 TRUE,
>> +                                 G_PARAM_READABLE |
>> +                                 G_PARAM_STATIC_STRINGS);
>> +    g_object_class_install_property(g_klass,
>> +                                    PROP_ALPHA,
>> +                                    pspec);
>> +}
>> +
>> +static void
>> +osinfo_avatar_format_init (OsinfoAvatarFormat *avatar)
>> +{
>> +}
>> +
>> +/**
>> + * osinfo_avatar_format_new:
>> + *
>> + * Construct a new user avatar file to a #OsinfoInstallScript.
>> + *
>> + * Returns: (transfer full): the necessary information to create an avatar for
>> + *                           an user
>> + */
>> +OsinfoAvatarFormat *
>> +osinfo_avatar_format_new(void)
>> +{
>> +    return g_object_new(OSINFO_TYPE_AVATAR_FORMAT, NULL);
>> +}
>> +
>> +/**
>> + * osinfo_avatar_format_get_mime_types:
>> + * @avatar: the avatar info
>> + *
>> + * Returns: (transfer container) (element-type utf8): the required mime-types of the avatar.
>> + */
>> +GList *
>> +osinfo_avatar_format_get_mime_types(OsinfoAvatarFormat *avatar)
>> +{
>> +    return osinfo_entity_get_param_value_list(OSINFO_ENTITY(avatar),
>> +                                              OSINFO_AVATAR_FORMAT_PROP_MIME_TYPE);
>> +}
>> +
>> +/**
>> + * osinfo_avatar_format_get_width:
>> + * @avatar: the avatar info
>> + *
>> + * Returns: the required width (in pixels) of the avatar, or -1.
>> + */
>> +gint
>> +osinfo_avatar_format_get_width(OsinfoAvatarFormat *avatar)
>> +{
>> +    return osinfo_entity_get_param_value_int64(OSINFO_ENTITY(avatar),
>> +                                               OSINFO_AVATAR_FORMAT_PROP_WIDTH);
>> +}
>> +
>> +/**
>> + * osinfo_avatar_format_get_height:
>> + * @avatar: the avatar info
>> + *
>> + * Returns: the required height (in pixels) of the avatar, or -1.
>> + */
>> +gint
>> +osinfo_avatar_format_get_height(OsinfoAvatarFormat *avatar)
>> +{
>> +    return osinfo_entity_get_param_value_int64(OSINFO_ENTITY(avatar),
>> +                                               OSINFO_AVATAR_FORMAT_PROP_HEIGHT);
>> +}
>> +
>> +/**
>> + * osinfo_avatar_format_get_alpha:
>> + * @avatar: the avatar info
>> + *
>> + * Returns: Whether alpha channel is supported in the avatar.
>> + */
>> +gboolean
>> +osinfo_avatar_format_get_alpha(OsinfoAvatarFormat *avatar)
>> +{
>> +    return osinfo_entity_get_param_value_boolean_with_default
>> +                (OSINFO_ENTITY(avatar), OSINFO_AVATAR_FORMAT_PROP_ALPHA, TRUE);
>> +}
>> +
>> +/*
>> + * Local variables:
>> + *  indent-tabs-mode: nil
>> + *  c-indent-level: 4
>> + *  c-basic-offset: 4
>> + * End:
>> + */
>> diff --git a/osinfo/osinfo_avatar_format.h b/osinfo/osinfo_avatar_format.h
>> new file mode 100644
>> index 0000000..fc4e03d
>> --- /dev/null
>> +++ b/osinfo/osinfo_avatar_format.h
>> @@ -0,0 +1,95 @@
>> +/*
>> + * libosinfo: OS installation avatar information
>> + *
>> + * Copyright (C) 2009-2012 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + *
>> + * Authors:
>> + *   Fabiano Fidêncio <fabiano at fidencio.org>
>> + *   Zeeshan Ali (Khattak) <zeeshanak at gnome.org>
>> + */
>> +
>> +#include <glib-object.h>
>> +
>> +#ifndef __OSINFO_AVATAR_FORMAT_H__
>> +#define __OSINFO_AVATAR_FORMAT_H__
>> +
>> +/*
>> + * Type macros.
>> + */
>> +#define OSINFO_TYPE_AVATAR_FORMAT              \
>> +        (osinfo_avatar_format_get_type ())
>> +
>> +#define OSINFO_AVATAR_FORMAT(obj)              \
>> +        (G_TYPE_CHECK_INSTANCE_CAST ((obj),    \
>> +         OSINFO_TYPE_AVATAR_FORMAT,            \
>> +         OsinfoAvatarFormat))
>> +
>> +#define OSINFO_IS_AVATAR_FORMAT(obj)           \
>> +        (G_TYPE_CHECK_INSTANCE_TYPE ((obj),    \
>> +         OSINFO_TYPE_AVATAR_FORMAT))
>> +
>> +#define OSINFO_AVATAR_FORMAT_CLASS(klass)      \
>> +        (G_TYPE_CHECK_CLASS_CAST ((klass),     \
>> +         OSINFO_TYPE_AVATAR_FORMAT,            \
>> +         OsinfoAvatarFormatClass))
>> +
>> +#define OSINFO_IS_AVATAR_FORMAT_CLASS(klass)   \
>> +        (G_TYPE_CHECK_CLASS_TYPE ((klass),     \
>> +         OSINFO_TYPE_AVATAR_FORMAT))
>> +
>> +#define OSINFO_AVATAR_FORMAT_GET_CLASS(obj)    \
>> +        (G_TYPE_INSTANCE_GET_CLASS ((obj),     \
>> +         OSINFO_TYPE_AVATAR_FORMAT,            \
>> +         OsinfoAvatarFormatClass))
>> +
>> +typedef struct _OsinfoAvatarFormat OsinfoAvatarFormat;
>> +typedef struct _OsinfoAvatarFormatClass OsinfoAvatarFormatClass;
>> +
>> +#define OSINFO_AVATAR_FORMAT_PROP_MIME_TYPE "mime-type"
>> +#define OSINFO_AVATAR_FORMAT_PROP_WIDTH     "width"
>> +#define OSINFO_AVATAR_FORMAT_PROP_HEIGHT    "height"
>> +#define OSINFO_AVATAR_FORMAT_PROP_ALPHA     "alpha"
>> +
>> +/* object */
>> +struct _OsinfoAvatarFormat
>> +{
>> +    OsinfoEntity parent_instance;
>> +};
>> +
>> +/* class */
>> +struct _OsinfoAvatarFormatClass
>> +{
>> +    OsinfoEntityClass parent_class;
>> +};
>> +
>> +GType osinfo_avatar_format_get_type(void);
>> +
>> +OsinfoAvatarFormat *osinfo_avatar_format_new(void);
>> +
>> +GList *osinfo_avatar_format_get_mime_types(OsinfoAvatarFormat *avatar);
>> +gint osinfo_avatar_format_get_width(OsinfoAvatarFormat *avatar);
>> +gint osinfo_avatar_format_get_height(OsinfoAvatarFormat *avatar);
>> +gboolean osinfo_avatar_format_get_alpha(OsinfoAvatarFormat *avatar);
>> +
>> +#endif /* __OSINFO_AVATAR_FORMAT_H__ */
>> +/*
>> + * Local variables:
>> + *  indent-tabs-mode: nil
>> + *  c-indent-level: 4
>> + *  c-basic-offset: 4
>> + * End:
>> + */
>> diff --git a/osinfo/osinfo_install_script.c b/osinfo/osinfo_install_script.c
>> index 54dae64..9751e47 100644
>> --- a/osinfo/osinfo_install_script.c
>> +++ b/osinfo/osinfo_install_script.c
>> @@ -50,6 +50,7 @@ struct _OsinfoInstallScriptPrivate
>>      gchar *output_prefix;
>>      gchar *output_filename;
>>      GList *config_param_list;
>> +    OsinfoAvatarFormat *avatar;
>>  };
>>
>>  enum {
>> @@ -60,6 +61,7 @@ enum {
>>      PROP_PROFILE,
>>      PROP_PRODUCT_KEY_FORMAT,
>>      PROP_PATH_FORMAT,
>> +    PROP_AVATAR_FORMAT,
>>  };
>>
>>  typedef struct _OsinfoInstallScriptGenerateData OsinfoInstallScriptGenerateData;
>> @@ -142,6 +144,11 @@ osinfo_os_get_property(GObject    *object,
>>                           osinfo_install_script_get_path_format(script));
>>          break;
>>
>> +    case PROP_AVATAR_FORMAT:
>> +        g_value_take_object(value,
>> +                            osinfo_install_script_get_avatar_format(script));
>> +        break;
>> +
>>      default:
>>          /* We don't have any other property... */
>>          G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
>> @@ -157,6 +164,8 @@ osinfo_install_script_finalize (GObject *object)
>>      g_free(script->priv->output_prefix);
>>      g_free(script->priv->output_filename);
>>      g_list_free_full(script->priv->config_param_list, g_object_unref);
>> +    if (script->priv->avatar != NULL)
>> +        g_object_unref(script->priv->avatar);
>>
>>      /* Chain up to the parent class */
>>      G_OBJECT_CLASS (osinfo_install_script_parent_class)->finalize (object);
>> @@ -240,6 +249,16 @@ osinfo_install_script_class_init (OsinfoInstallScriptClass *klass)
>>                                      PROP_PATH_FORMAT,
>>                                      pspec);
>>
>> +    pspec = g_param_spec_object("avatar-format",
>> +                                "Avatar Format",
>> +                                _("Expected avatar format"),
>> +                                OSINFO_TYPE_AVATAR_FORMAT,
>> +                                G_PARAM_READABLE |
>> +                                G_PARAM_STATIC_STRINGS);
>> +    g_object_class_install_property(g_klass,
>> +                                    PROP_AVATAR_FORMAT,
>> +                                    pspec);
>> +
>>      g_type_class_add_private (klass, sizeof (OsinfoInstallScriptPrivate));
>>  }
>>
>> @@ -464,6 +483,38 @@ const gchar *osinfo_install_script_get_output_filename(OsinfoInstallScript *scri
>>      return script->priv->output_filename;
>>  }
>>
>> +void
>> +osinfo_install_script_set_avatar_format(OsinfoInstallScript *script,
>> +                                        OsinfoAvatarFormat *avatar)
>> +{
>> +    g_return_if_fail(OSINFO_IS_INSTALL_SCRIPT(script));
>> +    g_return_if_fail(OSINFO_IS_AVATAR_FORMAT(avatar));
>> +
>> +    if (script->priv->avatar != NULL)
>> +        g_object_unref(script->priv->avatar);
>> +    script->priv->avatar = g_object_ref(avatar);
>
>> +}
>> +
>> +/**
>> + * osinfo_install_script_get_avatar_format
>> + *
>> + * Some install scripts have restrictions on the format of the user avatar. Use
>> + * this method to retrieve those restrictions in the form of an
>> + * #OsinfoAvatarFormat instance.
>> + *
>> + * Returns: (transfer full): The avatar format, or NULL if there is no restrictions on the
>> + *                           format of avatar
>> + */
>> +OsinfoAvatarFormat *osinfo_install_script_get_avatar_format(OsinfoInstallScript *script)
>> +{
>> +    g_return_val_if_fail(OSINFO_IS_INSTALL_SCRIPT(script), NULL);
>> +
>> +    if (script->priv->avatar == NULL)
>> +        return NULL;
>> +
>> +    return g_object_ref(script->priv->avatar);
>
> The libosinfo object getters I looked at are (transfer none), any specific reason for
> this one to be (transfer full) ?

We already have some exception to this in libosinfo, especially in
case of lists and list objects. The general practice in the gobject
world (I think I actually read this in gobject docs) is to always
return a new ref for objects so I usually just follow that unless
project in question has an explicit rule to not follow it. What do you
think we should do here?

>> +}
>> +
>>  struct _OsinfoInstallScriptGenerateData {
>>      GSimpleAsyncResult *res;
>>      OsinfoOs *os;
>> diff --git a/osinfo/osinfo_install_script.h b/osinfo/osinfo_install_script.h
>> index 036b572..c6bc2df 100644
>> --- a/osinfo/osinfo_install_script.h
>> +++ b/osinfo/osinfo_install_script.h
>> @@ -24,6 +24,7 @@
>>  #include <glib-object.h>
>>  #include <gio/gio.h>
>>  #include <osinfo/osinfo_install_config_param.h>
>> +#include <osinfo/osinfo_avatar_format.h>
>>
>>  #ifndef __OSINFO_INSTALL_SCRIPT_H__
>>  #define __OSINFO_INSTALL_SCRIPT_H__
>> @@ -106,6 +107,11 @@ const gchar *osinfo_install_script_get_output_filename(OsinfoInstallScript *scri
>>
>>  const gchar *osinfo_install_script_get_expected_filename(OsinfoInstallScript *script);
>>
>> +void osinfo_install_script_set_avatar_format(OsinfoInstallScript *script,
>> +                                             OsinfoAvatarFormat *avatar);
>> +
>
> This symbol is not exported so should not appear in a public header.

Actually I forgot to export it. :) But now that you mention it, yeah I
think we should put it in a private header..


-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124




More information about the virt-tools-list mailing list