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