[PATCH v6] backends/cryptodev: drop local_err from cryptodev_backend_complete()

Vladimir Sementsov-Ogievskiy posted 1 patch 4 years, 4 months ago
Test asan passed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191127194541.24304-1-vsementsov@virtuozzo.com
backends/cryptodev.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
[PATCH v6] backends/cryptodev: drop local_err from cryptodev_backend_complete()
Posted by Vladimir Sementsov-Ogievskiy 4 years, 4 months ago
No reason for local_err here, use errp directly instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

v6: add r-b by Philippe and Marc-André

 backends/cryptodev.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 3c071eab95..5a9735684e 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -176,19 +176,10 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
 {
     CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
     CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
-    Error *local_err = NULL;
 
     if (bc->init) {
-        bc->init(backend, &local_err);
-        if (local_err) {
-            goto out;
-        }
+        bc->init(backend, errp);
     }
-
-    return;
-
-out:
-    error_propagate(errp, local_err);
 }
 
 void cryptodev_backend_set_used(CryptoDevBackend *backend, bool used)
-- 
2.21.0


Re: [PATCH v6] backends/cryptodev: drop local_err from cryptodev_backend_complete()
Posted by Markus Armbruster 4 years, 4 months ago
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> No reason for local_err here, use errp directly instead.

Related: "[PATCH v6] hw/vfio/ap: drop local_err from vfio_ap_realize".
I'm surprised it's just two.  Did you search for the anti-pattern
systematically?

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>
> v6: add r-b by Philippe and Marc-André
>
>  backends/cryptodev.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index 3c071eab95..5a9735684e 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -176,19 +176,10 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
>  {
>      CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
>      CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
> -    Error *local_err = NULL;
>  
>      if (bc->init) {
> -        bc->init(backend, &local_err);
> -        if (local_err) {
> -            goto out;
> -        }
> +        bc->init(backend, errp);
>      }
> -
> -    return;
> -
> -out:
> -    error_propagate(errp, local_err);
>  }
>  
>  void cryptodev_backend_set_used(CryptoDevBackend *backend, bool used)

Reviewed-by: Markus Armbruster <armbru@redhat.com>


Re: [PATCH v6] backends/cryptodev: drop local_err from cryptodev_backend_complete()
Posted by Vladimir Sementsov-Ogievskiy 4 years, 4 months ago
29.11.2019 9:10, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> 
>> No reason for local_err here, use errp directly instead.
> 
> Related: "[PATCH v6] hw/vfio/ap: drop local_err from vfio_ap_realize".
> I'm surprised it's just two.  Did you search for the anti-pattern
> systematically?

As I remember - no, only where it breaks automation. Here empty "out: }"
label will break compilation, if not fixed before or after coccinelle
run. So, here I fixed it before.

> 
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>
>> v6: add r-b by Philippe and Marc-André
>>
>>   backends/cryptodev.c | 11 +----------
>>   1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
>> index 3c071eab95..5a9735684e 100644
>> --- a/backends/cryptodev.c
>> +++ b/backends/cryptodev.c
>> @@ -176,19 +176,10 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
>>   {
>>       CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
>>       CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
>> -    Error *local_err = NULL;
>>   
>>       if (bc->init) {
>> -        bc->init(backend, &local_err);
>> -        if (local_err) {
>> -            goto out;
>> -        }
>> +        bc->init(backend, errp);
>>       }
>> -
>> -    return;
>> -
>> -out:
>> -    error_propagate(errp, local_err);
>>   }
>>   
>>   void cryptodev_backend_set_used(CryptoDevBackend *backend, bool used)
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 


-- 
Best regards,
Vladimir
Re: [PATCH v6] backends/cryptodev: drop local_err from cryptodev_backend_complete()
Posted by Markus Armbruster 4 years, 4 months ago
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> 29.11.2019 9:10, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>> 
>>> No reason for local_err here, use errp directly instead.
>> 
>> Related: "[PATCH v6] hw/vfio/ap: drop local_err from vfio_ap_realize".
>> I'm surprised it's just two.  Did you search for the anti-pattern
>> systematically?
>
> As I remember - no, only where it breaks automation. Here empty "out: }"
> label will break compilation, if not fixed before or after coccinelle
> run. So, here I fixed it before.

Got it.

A through sweep would be nice, but the goal you're pursuing here
involves more than enough churn already.  Don't worry about it now.

[...]


RE: [PATCH v6] backends/cryptodev: drop local_err from cryptodev_backend_complete()
Posted by Gonglei (Arei) 4 years, 4 months ago
CCing qemu-trivial@nongnu.org

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


Regards,
-Gonglei

> -----Original Message-----
> From: Vladimir Sementsov-Ogievskiy [mailto:vsementsov@virtuozzo.com]
> Sent: Thursday, November 28, 2019 3:46 AM
> To: qemu-devel@nongnu.org
> Cc: Gonglei (Arei) <arei.gonglei@huawei.com>; marcandre.lureau@gmail.com;
> philmd@redhat.com; vsementsov@virtuozzo.com
> Subject: [PATCH v6] backends/cryptodev: drop local_err from
> cryptodev_backend_complete()
> 
> No reason for local_err here, use errp directly instead.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 
> v6: add r-b by Philippe and Marc-André
> 
>  backends/cryptodev.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c index
> 3c071eab95..5a9735684e 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -176,19 +176,10 @@ cryptodev_backend_complete(UserCreatable *uc,
> Error **errp)  {
>      CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
>      CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
> -    Error *local_err = NULL;
> 
>      if (bc->init) {
> -        bc->init(backend, &local_err);
> -        if (local_err) {
> -            goto out;
> -        }
> +        bc->init(backend, errp);
>      }
> -
> -    return;
> -
> -out:
> -    error_propagate(errp, local_err);
>  }
> 
>  void cryptodev_backend_set_used(CryptoDevBackend *backend, bool used)
> --
> 2.21.0