Re: [libvirt] [PATCH] Fixed URI parsing
On 02/09/2012 04:38 PM, Laine Stump wrote:
On 02/09/2012 09:43 AM, Martin Kletzander wrote:
> Function virParseURI was added. This function is wrapper around
> xmlParseURI. In this wrapper we are fixing what doesn't seems to be
> fixed in libxml2 in the near future. The wrapper is written in such
> way that if anything gets fixed in libxml2, it'll still work.
The problem alluded to here is that when xmlParseURI encounters a
numeric IPv6 address in brackets, it returns the entire string including
the brackets. getaddrxx(), by contrast, expects the brackets to not be
there. And yet the brackets *must* be included to specify a numeric IP
address, according to section 6 of RFC5952 (why am I ready with this
information? Because I looked it up when commenting on a qemu bug last
night :-)
(https://bugzilla.redhat.com/show_bug.cgi?id=680356 if you're interested)
I'll definitely check this out, thanks for the info.
I almost ACKed this patch (with one small change to replace the loop
with memmove), but then thought about what happens if we need to
reconstruct the URI from this modified xmlURIPtr (e.g., with xmlSaveUri,
which libvirt calls in two places).
Jiri told me about this, it's just my fault that I've forgot to modify
that part as well. What is your opinion for me setting up one more
function (virSaveURI let's say) that constructs the string back as it is
supposed to (again just a wrapper for xmlSaveURI)?
So, I think that the correct solution here, rather than permanently
modifying the server string returned from xmlParseURI, is to look at the
places where the server string is used for something other than just
checking for != NULL (that leaves very few places) and modify a copy of
the string to remove the brackets in those cases. This way you always
have the exact original server string so that the original URI can be
reconstructed.
That was the first thing I wanted to change but unfortunately I did that
only for ssh so it was unusable.
> + for (size_t i = 0; i< last; i++)
> + ret->server[i] = ret->server[i + 1];
(this actually copies one character too many, but that's harmless). Just
replace this with:
memmove(ret->server, ret->server+1, last-1)
I know about memmove(), it's just that in this case I really didn't like
that it copes the string somewhere else and than again back (in case of
overlapping) but no problem to change this :-)