On Fri, Oct 1, 2021 at 8:28 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote: > > On Fri, Oct 01, 2021 at 01:13:00PM -0400, Laine Stump wrote: > > On 10/1/21 5:57 AM, Daniel P. Berrangé wrote: > > > On Mon, Sep 27, 2021 at 10:30:47PM +0300, Dmitrii Shcherbakov wrote: > > > > [...] > > > > > +GType > > > > +vir_pci_vpd_resource_type_get_type(void) > > > > I know you had asked about using under_scored_naming in a reply to Peter > > pointing out "non-standard" names in V3 of your patches, but I don't think > > anyone followed up on that. > > That very specific case is something that is required when we use > GObject for the type. see util/viridentity.h for the same scenario. > There are a couple of other similar requirements force on us by > GObject in this regard, but aside from that we should follow > normal libvirt naming practice. Yes, those names are enforced in the glib macros: https://github.com/GNOME/glib/blob/2.53.7/gobject/gtype.h#L1396 GType module_obj_name##_get_type (void); \ https://github.com/GNOME/glib/blob/2.53.7/gobject/gtype.h#L1949-L1951 static void type_name##_init (TypeName *self); \ static void type_name##_class_init (TypeName##Class *klass); \ So there isn't much choice. But Peter was right about property getters/setters and the finalize function - for those glib merely needs function pointers and Libvirt naming conventions can be used there. > > > > > +static gboolean > > > > In a similar "coding standards" vein - other uses of "gboolean" (rather than > > the much more common "bool", or *very rarely* "boolean") in our code seem to > > be the product of auto-generated(?) xdr headers for wireshark, functions > > that are registered as callbacks to glib (and so must follow the types in > > the callback function pointer prototype), and a very small number of other > > cases. Do we want new code using gboolean rather than bool in these "other" > > cases that don't require gboolean for proper glib type compatibility? > > gboolean is a completely differnt sized type from bool. > > As a general rule we want to use 'bool' + true/false, except > where we must have strict type compatibility with glib. The > main scenario that matters is callbacks integrating with > GLib's event loop or similar. > > All the other gNNNN basic types are essentially identical > to stdint.h types, so there's no compelling reason to use > them. We can use the plain C types, or the stdint.h sized > types as appropriate and they'll be fully compatible with > any GLib APIs. > > > (a side-nit completely unrelated to your patches - I noticed that in at > > least a couple places in libvirt, a gboolean is assigned "true" or "false", > > although the glib documentation says that it should only have the value > > "TRUE" or "FALSE". Good thing those happen to use the same values!) > > We're lucky in that the constants do the right thing if > you assign/compare. eg a > > bool foo= TRUE; > if (foo == TRUE) > .. > > will be ok. > > I'm not so sure about > > bool foo = TRUE > gboolean bar == TRUE; > if (foo == bar) > ... > > because they're different types, and so when you size-extend > I think they end up with different values, despite both being > TRUE. > Thanks a lot for raising and discussing this - I was confused as to which types to preferably use as well.
