[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 for-2.12 24/25] block/curl: Implement bdrv_re
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH v7 for-2.12 24/25] block/curl: Implement bdrv_refresh_filename() |
Date: |
Fri, 08 Dec 2017 17:14:19 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Fri 08 Dec 2017 02:47:52 PM CET, Max Reitz <address@hidden> wrote:
>>>>> +static void curl_refresh_filename(BlockDriverState *bs)
>>>>> +{
>>>>> + BDRVCURLState *s = bs->opaque;
>>>>> +
>>>>> + if (!s->sslverify || s->cookie ||
>>>>> + s->username || s->password || s->proxyusername ||
>>>>> s->proxypassword)
>>>>> + {
>>>>
>>>> Is !s->sslverify negative because that setting is true by default?
>>>
>>> Yes, exactly. If it's false, you'd need to override it (and you can't
>>> do that through a plain filename).
>>
>> I think this is not the only case in this series, but I'm not very
>> comfortable with the idea that this condition and the default value
>> of the setting are implicity dependent on each other. If you change
>> one and forget to change the other things will break.
>
> Well, yes, but...
>
>> I understand that the default value is never supposed to change so in
>> practice I don't see this breaking,
>
> Yes.
>
>> but is it perhaps worth adding tests for all these cases?
>
> In theory, sure. In practice, adding a curl test case seems hard.
Indeed, I though it would perhaps be possible to create a curl BDS to
this without having to perform an actual connection, but never mind if
it's not possible / too complicated.
> Also, adding macros for the default values could help, I think.
Yep.
Berto