[libvirt] [PATCH v3 08/11] admin: Add support for URI aliases

[Martin Kletzander]

On Thu, Nov 19, 2015 at 02:00:16PM +0100, Erik Skultety wrote:
>>> -    if (!(conn->uri = virURIParse(uri ? uri : default_uri)))
>>> +    if ((!(flags & VIR_CONNECT_NO_ALIASES) &&
>>> +         virURIResolveAlias(conf, uri ? uri : default_uri, &alias) < 0))
>>
>> this should also be fixed (with what I mentioned in previous review).
>>
>
>Fixed.
>
>>> +        goto error;
>>> +
>>> +    if (!(conn->uri = virURIParse(alias ? alias : (uri ? uri :
>>> default_uri))))
>>
>> Also, if you modify virURIResolveAlias() to simply copy the string
>> over to @alias if the alias is not found, that would be pretty nice
>> syntax-sugar and spare our souls from this ugly thing =)
>>
>> ACK with that fixed.
>
>Hmm, I think that in ideal world 1 API should do exactly 1 thing and 1
>thing only, but we're not living in an ideal world, do we?! However, in
>this specific case I don't think it's worth changing alias resolving in
>a way that would copy the searched alias into URI string if the alias
>wasn't matched. I could do a wrapper encompassing the alias resolving
>and this proposed syntactic sugar, but it still wouldn't make it worth I
>guess, since the resolver isn't executed every time unless appropriate
>flags have been set. Moreover, since I fixed the ternary operator you
>suggested above, the other one also becomes more readable:
>virURIParse(alias ? alias : uri)....what do you think?
>

Well, one is better than two in this case, so at least one is fixed.
You don't need to change that just for the purpose of creating another
syntactic sugar, that was just a hint if you would like to do that.

>PS: thanks for the ACK though
>
>Erik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151123/ff3b9030/attachment-0001.sig>