RE: [PATCH 1/2] qom/object_interfaces: Remove unnecessary local_err check

2024-03-15 Thread Duan, Zhenzhong



>-Original Message-
>From: Liu, Zhao1 
>Subject: Re: [PATCH 1/2] qom/object_interfaces: Remove unnecessary
>local_err check
>
>On Thu, Feb 29, 2024 at 11:37:38AM +0800, Zhenzhong Duan wrote:
>> Date: Thu, 29 Feb 2024 11:37:38 +0800
>> From: Zhenzhong Duan 
>> Subject: [PATCH 1/2] qom/object_interfaces: Remove unnecessary
>local_err
>>  check
>> X-Mailer: git-send-email 2.34.1
>>
>> In the error return path, local_err is always set, no need to check it.
>
>The original error handling code indicates "local_err is always set",
>and error_propagate() can handle the case that local_err is NULL.

Will do.

>
>> Signed-off-by: Zhenzhong Duan 
>> ---
>>  qom/object_interfaces.c | 10 --
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
>> index e0833c8bfe..255a7bf659 100644
>> --- a/qom/object_interfaces.c
>> +++ b/qom/object_interfaces.c
>> @@ -128,13 +128,11 @@ Object *user_creatable_add_type(const char
>*type, const char *id,
>>  }
>>  goto out;
>>  }
>> -out:
>> -if (local_err) {
>> -error_propagate(errp, local_err);
>> -object_unref(obj);
>> -return NULL;
>> -}
>>  return obj;
>> +out:
>
>Maybe rename this to "err:"? Since now it's just used to handle error,
>and "goto err" seems more clear.

Good suggestion, will do.

Thanks
Zhenzhong

>
>> +error_propagate(errp, local_err);
>> +object_unref(obj);
>> +return NULL;
>>  }
>>
>>  void user_creatable_add_qapi(ObjectOptions *options, Error **errp)
>> --
>> 2.34.1
>>
>
>Otherwise,
>
>Reviewed-by: Zhao Liu 
>




Re: [PATCH 1/2] qom/object_interfaces: Remove unnecessary local_err check

2024-03-15 Thread Zhao Liu
On Thu, Feb 29, 2024 at 11:37:38AM +0800, Zhenzhong Duan wrote:
> Date: Thu, 29 Feb 2024 11:37:38 +0800
> From: Zhenzhong Duan 
> Subject: [PATCH 1/2] qom/object_interfaces: Remove unnecessary local_err
>  check
> X-Mailer: git-send-email 2.34.1
> 
> In the error return path, local_err is always set, no need to check it.

The original error handling code indicates "local_err is always set",
and error_propagate() can handle the case that local_err is NULL.

> Signed-off-by: Zhenzhong Duan 
> ---
>  qom/object_interfaces.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index e0833c8bfe..255a7bf659 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -128,13 +128,11 @@ Object *user_creatable_add_type(const char *type, const 
> char *id,
>  }
>  goto out;
>  }
> -out:
> -if (local_err) {
> -error_propagate(errp, local_err);
> -object_unref(obj);
> -return NULL;
> -}
>  return obj;
> +out:

Maybe rename this to "err:"? Since now it's just used to handle error,
and "goto err" seems more clear.

> +error_propagate(errp, local_err);
> +object_unref(obj);
> +return NULL;
>  }
>  
>  void user_creatable_add_qapi(ObjectOptions *options, Error **errp)
> -- 
> 2.34.1
> 

Otherwise,

Reviewed-by: Zhao Liu