[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/3] esx: Wrap libcurl multi handle



On Mon, Jul 2, 2012 at 4:44 PM, Matthias Bolte
<matthias bolte googlemail com> wrote:
> ---
>  src/esx/esx_vi.c |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/esx/esx_vi.h |   18 +++++++++
>  2 files changed, 129 insertions(+), 0 deletions(-)
>
> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index 5b5ab69..48718b6 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -86,6 +86,7 @@ ESX_VI__TEMPLATE__ALLOC(CURL)
>  ESX_VI__TEMPLATE__FREE(CURL,
>  {
>      esxVI_SharedCURL *shared = item->shared;
> +    esxVI_MultiCURL *multi = item->multi;
>
>      if (shared != NULL) {
>          esxVI_SharedCURL_Remove(shared, item);
> @@ -95,6 +96,14 @@ ESX_VI__TEMPLATE__FREE(CURL,
>          }
>      }
>
> +    if (multi != NULL) {
> +        esxVI_MultiCURL_Remove(multi, item);
> +
> +        if (multi->count == 0) {
> +            esxVI_MultiCURL_Free(&multi);
> +        }
> +    }
> +
>      if (item->handle != NULL) {
>          curl_easy_cleanup(item->handle);
>      }
> @@ -555,11 +564,15 @@ esxVI_SharedCURL_Add(esxVI_SharedCURL *shared, esxVI_CURL *curl)
>          }
>      }
>
> +    virMutexLock(&curl->lock);
> +
>      curl_easy_setopt(curl->handle, CURLOPT_SHARE, shared->handle);
>
>      curl->shared = shared;
>      ++shared->count;
>
> +    virMutexUnlock(&curl->lock);
> +
>      return 0;
>  }
>
> @@ -583,11 +596,109 @@ esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl)
>          return -1;
>      }
>
> +    virMutexLock(&curl->lock);
> +
>      curl_easy_setopt(curl->handle, CURLOPT_SHARE, NULL);
>
>      curl->shared = NULL;
>      --shared->count;
>
> +    virMutexUnlock(&curl->lock);
> +
> +    return 0;
> +}
> +
> +
> +
> +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> + * MultiCURL
> + */
> +
> +/* esxVI_MultiCURL_Alloc */
> +ESX_VI__TEMPLATE__ALLOC(MultiCURL)
> +
> +/* esxVI_MultiCURL_Free */
> +ESX_VI__TEMPLATE__FREE(MultiCURL,
> +{
> +    if (item->count > 0) {
> +        /* Better leak than crash */
> +        VIR_ERROR(_("Trying to free MultiCURL object that is still in use"));
> +        return;
> +    }
> +
> +    if (item->handle != NULL) {
> +        curl_multi_cleanup(item->handle);
> +    }

Since its a double pointer maybe setting the passed in value to NULL
to prevent a double free situations?

> +})
> +
> +int
> +esxVI_MultiCURL_Add(esxVI_MultiCURL *multi, esxVI_CURL *curl)
> +{
> +    if (curl->handle == NULL) {
> +        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
> +                     _("Cannot add uninitialized CURL handle to a multi handle"));
> +        return -1;
> +    }
> +
> +    if (curl->multi != NULL) {
> +        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
> +                     _("Cannot add CURL handle to a multi handle twice"));
> +        return -1;
> +    }
> +
> +    if (multi->handle == NULL) {
> +        multi->handle = curl_multi_init();
> +
> +        if (multi->handle == NULL) {
> +            ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
> +                         _("Could not initialize CURL (multi)"));
> +            return -1;
> +        }
> +    }
> +
> +    virMutexLock(&curl->lock);
> +
> +    curl_multi_add_handle(multi->handle, curl->handle);
> +
> +    curl->multi = multi;
> +    ++multi->count;
> +
> +    virMutexUnlock(&curl->lock);
> +
> +    return 0;
> +}
> +
> +int
> +esxVI_MultiCURL_Remove(esxVI_MultiCURL *multi, esxVI_CURL *curl)
> +{
> +    if (curl->handle == NULL) {
> +        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
> +                     _("Cannot remove uninitialized CURL handle from a "
> +                       "multi handle"));
> +        return -1;
> +    }
> +
> +    if (curl->multi == NULL) {
> +        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
> +                     _("Cannot remove CURL handle from a multi handle when it "
> +                       "wasn't added before"));
> +        return -1;
> +    }
> +
> +    if (curl->multi != multi) {
> +        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("CURL (multi) mismatch"));
> +        return -1;
> +    }
> +
> +    virMutexLock(&curl->lock);
> +
> +    curl_multi_remove_handle(multi->handle, curl->handle);
> +
> +    curl->multi = NULL;
> +    --multi->count;
> +
> +    virMutexUnlock(&curl->lock);

Maybe add your free code here when count is 0? That way you wouldn't
have to contend with a potential memory leak case when the free is
called when its still ref'd.

> +
>      return 0;
>  }
>
> diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h
> index 78d3986..9560bd2 100644
> --- a/src/esx/esx_vi.h
> +++ b/src/esx/esx_vi.h
> @@ -85,6 +85,7 @@ typedef enum _esxVI_Occurrence esxVI_Occurrence;
>  typedef struct _esxVI_ParsedHostCpuIdInfo esxVI_ParsedHostCpuIdInfo;
>  typedef struct _esxVI_CURL esxVI_CURL;
>  typedef struct _esxVI_SharedCURL esxVI_SharedCURL;
> +typedef struct _esxVI_MultiCURL esxVI_MultiCURL;
>  typedef struct _esxVI_Context esxVI_Context;
>  typedef struct _esxVI_Response esxVI_Response;
>  typedef struct _esxVI_Enumeration esxVI_Enumeration;
> @@ -160,6 +161,7 @@ struct _esxVI_CURL {
>      struct curl_slist *headers;
>      char error[CURL_ERROR_SIZE];
>      esxVI_SharedCURL *shared;
> +    esxVI_MultiCURL *multi;
>  };
>
>  int esxVI_CURL_Alloc(esxVI_CURL **curl);
> @@ -188,6 +190,22 @@ int esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl);
>
>
>  /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> + * MultiCURL
> + */
> +
> +struct _esxVI_MultiCURL {
> +    CURLM *handle;
> +    size_t count;
> +};
> +
> +int esxVI_MultiCURL_Alloc(esxVI_MultiCURL **multi);
> +void esxVI_MultiCURL_Free(esxVI_MultiCURL **multi);
> +int esxVI_MultiCURL_Add(esxVI_MultiCURL *multi, esxVI_CURL *curl);
> +int esxVI_MultiCURL_Remove(esxVI_MultiCURL *multi, esxVI_CURL *curl);
> +
> +
> +
> +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>   * Context
>   */
>
> --
> 1.7.4.1
>
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list



-- 
Doug Goldstein


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]