diff mbox

[1/7] curl: strengthen assertion in curl_clean_state

Message ID 20170510143205.32013-2-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini May 10, 2017, 2:31 p.m. UTC
curl_clean_state should only be called after all AIOCBs have been
completed.  This is not so obvious for the call from curl_detach_aio_context,
so assert that.

Cc: qemu-stable@nongnu.org
Cc: jcody@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/curl.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Max Reitz May 10, 2017, 4:33 p.m. UTC | #1
On 10.05.2017 16:31, Paolo Bonzini wrote:
> curl_clean_state should only be called after all AIOCBs have been
> completed.  This is not so obvious for the call from curl_detach_aio_context,
> so assert that.
> 
> Cc: qemu-stable@nongnu.org
> Cc: jcody@redhat.com
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/curl.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 2708d57c2f..25a301e7b4 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -532,6 +532,11 @@ static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s)
>  
>  static void curl_clean_state(CURLState *s)
>  {
> +    int j;
> +    for (j=0; j<CURL_NUM_ACB; j++) {

See checkpatch output, but apart from that:

Reviewed-by: Max Reitz <mreitz@redhat.com>

> +        assert(!s->acb[j]);
> +    }
> +
>      if (s->s->multi)
>          curl_multi_remove_handle(s->s->multi, s->curl);
>  
>
Jeff Cody May 11, 2017, 8:35 p.m. UTC | #2
On Wed, May 10, 2017 at 04:31:59PM +0200, Paolo Bonzini wrote:
> curl_clean_state should only be called after all AIOCBs have been
> completed.  This is not so obvious for the call from curl_detach_aio_context,
> so assert that.
> 
> Cc: qemu-stable@nongnu.org
> Cc: jcody@redhat.com
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/curl.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 2708d57c2f..25a301e7b4 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -532,6 +532,11 @@ static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s)
>  
>  static void curl_clean_state(CURLState *s)
>  {
> +    int j;
> +    for (j=0; j<CURL_NUM_ACB; j++) {
> +        assert(!s->acb[j]);
> +    }
> +
>      if (s->s->multi)
>          curl_multi_remove_handle(s->s->multi, s->curl);
>  
> -- 
> 2.12.2
> 
>

Minor formatting nit aside (if no other revisions needed, I can fix that on
apply):

Reviewed-by: Jeff Cody <jcody@redhat.com>
diff mbox

Patch

diff --git a/block/curl.c b/block/curl.c
index 2708d57c2f..25a301e7b4 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -532,6 +532,11 @@  static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s)
 
 static void curl_clean_state(CURLState *s)
 {
+    int j;
+    for (j=0; j<CURL_NUM_ACB; j++) {
+        assert(!s->acb[j]);
+    }
+
     if (s->s->multi)
         curl_multi_remove_handle(s->s->multi, s->curl);