[Libosinfo] [libosinfo PATCH 1/7] entity: Add methods to deal with GFlags

[Fabiano Fidêncio]

[snip]

> >
> > +void osinfo_entity_set_param_flags(OsinfoEntity *entity, const gchar *key, guint value, GType flags_type)
> > +{
> > +    GFlagsClass *flags_class;
> > +    guint i;
> > +
> > +    g_return_if_fail(G_TYPE_IS_FLAGS(flags_type));
> > +
> > +    flags_class = g_type_class_ref(flags_type);
> > +    for (i = 0; i < flags_class->n_values; i++) {
> > +        if ((flags_class->values[i].value & value) != 0) {
> > +            osinfo_entity_set_param(entity, key, flags_class->values[i].value_nick);
> > +            break;
> > +        }
>
> GFlag is a type for using an enum as a bitmask. As such the
> value can have multiple bits set. This code however breaks
> as soon as a single matching flag bit is found, and ignores
> all following bits.

Hmm. True. I'll fix this if we decide to take this path ...
>
> This doesn't feel right to me.
>
> Why do we need to use GFlag if we're not going to honour
> more than one bit being set ? Doesn't that just become
> an enum at that point

I agree. There's one thing thought that, IIRC, made me take this path.
I wasn't willing to add a new enum for the possible injection methods
and I cannot use the current OsinfoInstallScriptInjectionMethod enum
as it's treated as a GFlags. Here's the generated code for it:

 66 GType
 67 osinfo_install_script_injection_method_get_type (void)
 68 {
 69   static volatile gsize g_define_type_id__volatile = 0;
 70
 71   if (g_once_init_enter (&g_define_type_id__volatile))
 72     {
 73       static const GFlagsValue values[] = {
 74         { OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_CDROM,
"OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_CDROM", "cdrom" },
 75         { OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK,
"OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_DISK", "disk" },
 76         { OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_FLOPPY,
"OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_FLOPPY", "floppy" },
 77         { OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_INITRD,
"OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_INITRD", "initrd" },
 78         { OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_WEB,
"OSINFO_INSTALL_SCRIPT_INJECTION_METHOD_WEB", "web" },
 79         { 0, NULL, NULL }
 80       };
 81       GType g_define_type_id =
 82         g_flags_register_static (g_intern_static_string
("OsinfoInstallScriptInjectionMethod"), values);
 83       g_once_init_leave (&g_define_type_id__volatile, g_define_type_id);
 84     }
 85
 86   return g_define_type_id__volatile;
 87 }

So, here we basically have two ways of doing this:
- Using the pre-existent GFlags we have (and then fixing this code);
- Creating a new enum (that won't be a flag) and be happy with it.

What's your preference?
[snip]

Best Regards,
-- 
Fabiano Fidêncio