[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
_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo
