On 2017-12-05 11:31, Alberto Garcia wrote: > On Mon 04 Dec 2017 07:26:02 PM CET, Max Reitz 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. Well, I could connect to, I don't know, qemu.org or something and then just test things there (with the index used as a raw image), but if I add one test for the curl protocol, nobody is ever going to run them... And in theory, that's not how a curl test should work. In theory, you'd expect the user to set up a localhost server with some known root directory and then _make_test_img etc. would set up images there. But that way, really nobody is ever going to run them. And just adding a test for all protocols that then accesses qemu.org feels somehow wrong, too... Maybe if I add it to a network group, that wouldn't feel so bad. Also, adding macros for the default values could help, I think. Max
signature.asc
Description: OpenPGP digital signature