On Tue, Nov 20, 2018 at 09:45:23AM +0400, Marc-André Lureau wrote: > Hi > > On Mon, Nov 19, 2018 at 5:59 PM Daniel P. Berrangé <berra...@redhat.com> > wrote: > > > > When debugging QEMU it is often useful to put a breakpoint on the > > error_setg_internal method impl. > > > > Unfortunately the object_property_add / object_class_property_add > > methods call object_property_find / object_class_property_find methods > > to check if a property exists already before adding the new property. > > > > As a result there are a huge number of calls to error_setg_internal > > on startup of most QEMU commands, making it very painful to set a > > breakpoint on this method. > > > > This puts a minor optimization on the code so that we avoid calling > > error_setg() when errp is NULL. Functionally there's no difference > > since error_setg() is a no-op when errp is NULL, but this lets us > > use breakpoints in GDB in a practical way. > > > > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com> > > Or maybe change the error_setg* macros? After all, if errp is NULL, > it's an error qemu can handle.
That would mean that all callers would get short circuited which I don't think it is desirable in general. This QOM usage is special in that the calling code is using the error scenario as a means to trigger different code flow. IOW it doesn't consider it an error scenario at all. > > --- > > qom/object.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/qom/object.c b/qom/object.c > > index 547dcf97c3..ddd5e7a30e 100644 > > --- a/qom/object.c > > +++ b/qom/object.c > > @@ -1087,7 +1087,12 @@ ObjectProperty *object_property_find(Object *obj, > > const char *name, > > return prop; > > } > > > > - error_setg(errp, "Property '.%s' not found", name); > > + /* Optimized to avoid calling error_setg if errp == NULL > > + * otherwise every property add call hits error_setg > > + * making it impratical to set breakpoints in GDB */ > > + if (errp) { > > + error_setg(errp, "Property '.%s' not found", name); > > + } > > return NULL; > > } > > > > @@ -1133,7 +1138,10 @@ ObjectProperty > > *object_class_property_find(ObjectClass *klass, const char *name, > > } > > > > prop = g_hash_table_lookup(klass->properties, name); > > - if (!prop) { > > + /* Optimized to avoid calling error_setg if errp == NULL > > + * otherwise every property add call hits error_setg > > + * making it impratical to set breakpoints in GDB */ > > + if (!prop && errp) { > > error_setg(errp, "Property '.%s' not found", name); > > } > > return prop; > > -- > > 2.19.1 > > > > > > > -- > Marc-André Lureau Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|