On Fri 08 Dec 2017 02:47:52 PM CET, Max Reitz <mre...@redhat.com> 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