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 [email protected] +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
