Re: [libvirt] [PATCH v2] Fixed URI parsing
On 02/24/2012 06:30 AM, Martin Kletzander wrote:
Function xmlParseURI does not remove square brackets around IPv6
address when parsing. One of the solutions is making wrappers around
functions working with xmlURI*. This assures that uri->server will be
always properly assigned and it doesn't have to be changed when used
on some new place in the code.
For this purpose, functions virParseURI and virSaveURI were
added. These function are wrappers around xmlParseURI and xmlSaveUri
respectively.
+ */
+unsigned char *
+virSaveURI(xmlURIPtr uri)
+{
+ char *tmpserver, *backupserver = uri->server;
+ unsigned char *ret;
+ bool fixback = false;
Instead of using bool fixback, I'd set tmpserver as NULL here...
+
+ /* First check: does it make sense to do anything */
+ if (uri != NULL &&
+ uri->server != NULL &&
+ strchr(uri->server, ':') != NULL) {
+
+ /* To be sure we have enough space for the brackets, we save
+ * the old string and then alocate a new one */
s/alocate/allocate/ - but see below, as I don't think you need this comment.
+
+ /* the "+ 2" is room for square brackets and + 1 for '\0' */
+ size_t length = strlen(uri->server) + 3;
+
+ if(VIR_ALLOC_N(tmpserver, length) < 0) {
+ virReportOOMError();
No need to raise OOM error here - all callers of xmlSaveUri were already
raising their own OOM after a NULL return.
+ return NULL;
+ }
+
+ snprintf(tmpserver, length, "[%s]", uri->server);
I'd just use virAsnprintf(&tmpserver, "[%s]", uri->server); none of
this
need to call strlen, VIR_ALLOC_N, or snprintf.
+
+ uri->server = tmpserver;
+ bool fixback = true;
+ }
+
+ ret = xmlSaveUri(uri);
+
+ /* Put the fixed version back */
+ if (fixback) {
...and check 'if (tmpserver)' here (that is, fixback is redundant with
whether tmpserver was assigned at this point).
+ uri->server = backupserver;
+ VIR_FREE(tmpserver);
+ }
+
+ return ret;
+}
I'm also a bit worried that we might regress if we don't add a syntax
check; can you look at adding a rule to cfg.mk that poisons xmlParseURI
and xmlSaveUri (wow, libxml2 really does have the awful URI vs. Uri in
its naming conventions), then exempt util/uri.c from the check as the
only file allowed to use them.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)