| Message ID | 20240229033739.1123076-3-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Optimize user_creatable_add_type error path | expand |
On Thu, Feb 29, 2024 at 11:37:39AM +0800, Zhenzhong Duan wrote: > Date: Thu, 29 Feb 2024 11:37:39 +0800 > From: Zhenzhong Duan <zhenzhong.duan@intel.com> > Subject: [PATCH 2/2] qom/object_interfaces: Remove local_err in > user_creatable_add_type > X-Mailer: git-send-email 2.34.1 > > In user_creatable_add_type, there is mixed usage of ERRP_GUARD and > local_err. This makes error_abort not taking effect in those callee > functions with local_err passed. > > Now that we already has ERRP_GUARD, remove local_err and use *errp > instead. > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > qom/object_interfaces.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > index 255a7bf659..165cd433e7 100644 > --- a/qom/object_interfaces.c > +++ b/qom/object_interfaces.c > @@ -81,7 +81,6 @@ Object *user_creatable_add_type(const char *type, const char *id, > ERRP_GUARD(); > Object *obj; > ObjectClass *klass; > - Error *local_err = NULL; > > if (id != NULL && !id_wellformed(id)) { > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier"); > @@ -109,20 +108,20 @@ Object *user_creatable_add_type(const char *type, const char *id, > > assert(qdict); > obj = object_new(type); > - object_set_properties_from_qdict(obj, qdict, v, &local_err); > - if (local_err) { > + object_set_properties_from_qdict(obj, qdict, v, errp); It's better to make object_set_properties_from_qdict someting (e.g., boolean). Maybe an extra cleanup? > + if (*errp) { > goto out; > } > > if (id != NULL) { > object_property_try_add_child(object_get_objects_root(), > - id, obj, &local_err); > - if (local_err) { > + id, obj, errp); > + if (*errp) { > goto out; > } > } Here we could check whether the returned ObjectProperty* is NULL instaed of dereferencing errp. Thanks, Zhao
>-----Original Message----- >From: Liu, Zhao1 <zhao1.liu@intel.com> >Subject: Re: [PATCH 2/2] qom/object_interfaces: Remove local_err in >user_creatable_add_type > >On Thu, Feb 29, 2024 at 11:37:39AM +0800, Zhenzhong Duan wrote: >> Date: Thu, 29 Feb 2024 11:37:39 +0800 >> From: Zhenzhong Duan <zhenzhong.duan@intel.com> >> Subject: [PATCH 2/2] qom/object_interfaces: Remove local_err in >> user_creatable_add_type >> X-Mailer: git-send-email 2.34.1 >> >> In user_creatable_add_type, there is mixed usage of ERRP_GUARD and >> local_err. This makes error_abort not taking effect in those callee >> functions with local_err passed. >> >> Now that we already has ERRP_GUARD, remove local_err and use *errp >> instead. >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> --- >> qom/object_interfaces.c | 12 +++++------- >> 1 file changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c >> index 255a7bf659..165cd433e7 100644 >> --- a/qom/object_interfaces.c >> +++ b/qom/object_interfaces.c >> @@ -81,7 +81,6 @@ Object *user_creatable_add_type(const char *type, >const char *id, >> ERRP_GUARD(); >> Object *obj; >> ObjectClass *klass; >> - Error *local_err = NULL; >> >> if (id != NULL && !id_wellformed(id)) { >> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an >identifier"); >> @@ -109,20 +108,20 @@ Object *user_creatable_add_type(const char >*type, const char *id, >> >> assert(qdict); >> obj = object_new(type); >> - object_set_properties_from_qdict(obj, qdict, v, &local_err); >> - if (local_err) { >> + object_set_properties_from_qdict(obj, qdict, v, errp); > >It's better to make object_set_properties_from_qdict someting (e.g., >boolean). Maybe an extra cleanup? OK, will do. > >> + if (*errp) { >> goto out; >> } >> >> if (id != NULL) { >> object_property_try_add_child(object_get_objects_root(), >> - id, obj, &local_err); >> - if (local_err) { >> + id, obj, errp); >> + if (*errp) { >> goto out; >> } >> } > >Here we could check whether the returned ObjectProperty* is NULL instaed >of dereferencing errp. Indeed, that's better, will do. Thanks Zhenzhong
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index 255a7bf659..165cd433e7 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -81,7 +81,6 @@ Object *user_creatable_add_type(const char *type, const char *id, ERRP_GUARD(); Object *obj; ObjectClass *klass; - Error *local_err = NULL; if (id != NULL && !id_wellformed(id)) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier"); @@ -109,20 +108,20 @@ Object *user_creatable_add_type(const char *type, const char *id, assert(qdict); obj = object_new(type); - object_set_properties_from_qdict(obj, qdict, v, &local_err); - if (local_err) { + object_set_properties_from_qdict(obj, qdict, v, errp); + if (*errp) { goto out; } if (id != NULL) { object_property_try_add_child(object_get_objects_root(), - id, obj, &local_err); - if (local_err) { + id, obj, errp); + if (*errp) { goto out; } } - if (!user_creatable_complete(USER_CREATABLE(obj), &local_err)) { + if (!user_creatable_complete(USER_CREATABLE(obj), errp)) { if (id != NULL) { object_property_del(object_get_objects_root(), id); } @@ -130,7 +129,6 @@ Object *user_creatable_add_type(const char *type, const char *id, } return obj; out: - error_propagate(errp, local_err); object_unref(obj); return NULL; }
In user_creatable_add_type, there is mixed usage of ERRP_GUARD and local_err. This makes error_abort not taking effect in those callee functions with local_err passed. Now that we already has ERRP_GUARD, remove local_err and use *errp instead. Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- qom/object_interfaces.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)