[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 2/6] curl: Keep *socket until the end of curl_so
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH 2/6] curl: Keep *socket until the end of curl_sock_cb() |
Date: |
Tue, 10 Sep 2019 09:50:58 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 09.09.19 22:09, John Snow wrote:
>
>
> On 8/27/19 12:34 PM, Max Reitz wrote:
>> This does not really change anything, but it makes the code a bit easier
>> to follow once we use @socket as the opaque pointer for
>> aio_set_fd_handler().
>>
>> (Also, this change stops us from creating new CURLSocket objects when
>> the cURL library just wants to stop listening on an existing socket that
>> we do not recognize. With a well-behaving cURL, that should never
>> happen anyway.)
>>
>> Cc: address@hidden
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> block/curl.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/curl.c b/block/curl.c
>> index 92dc2f630e..8a45b371cc 100644
>> --- a/block/curl.c
>> +++ b/block/curl.c
>> @@ -174,18 +174,16 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd,
>> int action,
>> if (socket->fd == fd) {
>> if (action == CURL_POLL_REMOVE) {
>> QLIST_REMOVE(socket, next);
>> - g_free(socket);
>> }
>> break;
>> }
>> }
>
>> - if (!socket) {
>> + if (action != CURL_POLL_REMOVE && !socket) {
>> socket = g_new0(CURLSocket, 1);
>> socket->fd = fd;
>> socket->state = state;
>> QLIST_INSERT_HEAD(&state->sockets, socket, next);
>> }
>> - socket = NULL;
>>
>> trace_curl_sock_cb(action, (int)fd);
>> switch (action) {
>> @@ -207,6 +205,9 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd,
>> int action,
>> break;
>> }
>>
>> + if (action == CURL_POLL_REMOVE) {
>> + g_free(socket);
>> + }
>> return 0;
>> }
>>
>>
>
> Very naive question: why is CURL_POLL_REMOVE handled so early in the
> function? Why not handle both QLIST_REMOVE and g_free under the
> switch(action) construct entirely?
I don’t know how that’s a naive question, it’s just a different way to
approach this problem. Sure, we can do that. It is probably more
intuitive to everyone who hasn’t seen the state before this series.
Max
signature.asc
Description: OpenPGP digital signature