[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 5/7] curl: Report only ready sockets
From: |
Maxim Levitsky |
Subject: |
Re: [Qemu-block] [PATCH v2 5/7] curl: Report only ready sockets |
Date: |
Tue, 10 Sep 2019 19:12:29 +0300 |
On Tue, 2019-09-10 at 14:41 +0200, Max Reitz wrote:
> Instead of reporting all sockets to cURL, only report the one that has
> caused curl_multi_do_locked() to be called. This lets us get rid of the
> QLIST_FOREACH_SAFE() list, which was actually wrong: SAFE foreaches are
> only safe when the current element is removed in each iteration. If it
> possible for the list to be concurrently modified, we cannot guarantee
> that only the current element will be removed. Therefore, we must not
> use QLIST_FOREACH_SAFE() here.
>
> Fixes: ff5ca1664af85b24a4180d595ea6873fd3deac57
> Cc: address@hidden
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block/curl.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/block/curl.c b/block/curl.c
> index cf2686218d..fd70f1ebc4 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -392,24 +392,19 @@ static void curl_multi_check_completion(BDRVCURLState
> *s)
> }
>
> /* Called with s->mutex held. */
> -static void curl_multi_do_locked(CURLSocket *ready_socket)
> +static void curl_multi_do_locked(CURLSocket *socket)
Here you revert the variable name change you had in previous commit
> {
> - CURLSocket *socket, *next_socket;
> - CURLState *s = ready_socket->state;
> + BDRVCURLState *s = socket->state->s;
> int running;
> int r;
>
> - if (!s->s->multi) {
> + if (!s->multi) {
> return;
> }
>
> - /* Need to use _SAFE because curl_multi_socket_action() may trigger
> - * curl_sock_cb() which might modify this list */
> - QLIST_FOREACH_SAFE(socket, &s->sockets, next, next_socket) {
> - do {
> - r = curl_multi_socket_action(s->s->multi, socket->fd, 0,
> &running);
> - } while (r == CURLM_CALL_MULTI_PERFORM);
> - }
> + do {
> + r = curl_multi_socket_action(s->multi, socket->fd, 0, &running);
> + } while (r == CURLM_CALL_MULTI_PERFORM);
> }
>
> static void curl_multi_do(void *arg)
Other than that nitpick,
Reviewed-by: Maxim Levitsky <address@hidden>
Best regards,
Maxim Levitsky
- [Qemu-block] [PATCH v2 2/7] curl: Keep *socket until the end of curl_sock_cb(), (continued)
- [Qemu-block] [PATCH v2 2/7] curl: Keep *socket until the end of curl_sock_cb(), Max Reitz, 2019/09/10
- [Qemu-block] [PATCH v2 3/7] curl: Check completion in curl_multi_do(), Max Reitz, 2019/09/10
- [Qemu-block] [PATCH v2 4/7] curl: Pass CURLSocket to curl_multi_do(), Max Reitz, 2019/09/10
- [Qemu-block] [PATCH v2 5/7] curl: Report only ready sockets, Max Reitz, 2019/09/10
- Re: [Qemu-block] [PATCH v2 5/7] curl: Report only ready sockets,
Maxim Levitsky <=
- [Qemu-block] [PATCH v2 6/7] curl: Handle success in multi_check_completion, Max Reitz, 2019/09/10
- [Qemu-block] [PATCH v2 7/7] curl: Check curl_multi_add_handle()'s return code, Max Reitz, 2019/09/10
- Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/7] block/curl: Fix hang and potential crash, John Snow, 2019/09/10
- Re: [Qemu-block] [PATCH v2 0/7] block/curl: Fix hang and potential crash, Max Reitz, 2019/09/13