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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org