RE: [PATCH 2/2] qom/object_interfaces: Remove local_err in user_creatable_add_type

2024-03-15 Thread Duan, Zhenzhong



>-Original Message-
>From: Liu, Zhao1 
>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 
>> 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 
>> ---
>>  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



Re: [PATCH 2/2] qom/object_interfaces: Remove local_err in user_creatable_add_type

2024-03-15 Thread Zhao Liu
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 
> 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 
> ---
>  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