[Top][All Lists]

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

Re: [Qemu-block] [PATCH v2 3/7] curl: Check completion in curl_multi_do(

From: John Snow
Subject: Re: [Qemu-block] [PATCH v2 3/7] curl: Check completion in curl_multi_do()
Date: Tue, 10 Sep 2019 21:16:59 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0

On 9/10/19 12:11 PM, Maxim Levitsky wrote:
> On Tue, 2019-09-10 at 14:41 +0200, Max Reitz wrote:
>> While it is more likely that transfers complete after some file
>> descriptor has data ready to read, we probably should not rely on it.
>> Better be safe than sorry and call curl_multi_check_completion() in
>> curl_multi_do(), too, just like it is done in curl_multi_read().
>> With this change, curl_multi_do() and curl_multi_read() are actually the
>> same, so drop curl_multi_read() and use curl_multi_do() as the sole FD
>> handler.
> I understand the reasoning, but I still a bit worry that this
> could paper over some bug/race in the future.
> If curl asks us only to deal with write, that would mean
> that it doesn't expect any data to be received.
> Do you by a chance have an example, of this patch
> affecting the code? Maybe when a unexpected error reply
> is received from the server?
> I don't really know the CURL library, so I probably missed
> something important.
> Other than that,
> Reviewed-by: Maxim Levitsky <address@hidden>

In this case, it's because I had some doubts in V1 about when we call
the post-completion cleanup code. It didn't look obvious at a glance.

This just makes it simpler.

I don't think that by checking *more* things we're going to paper over
some race condition -- it's the opposite. If anything, this will explode
sooner rather than later.

So I think it's OK, naturally.

Reviewed-by: John Snow <address@hidden>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]