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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list