On Sun, Apr 03, 2011 at 11:21:22AM +0200, Matthias Bolte wrote:
> Removes multiple 4kb stack allocations.
> ---
> src/libvirt.c | 113 +++++++++++++++++++++------------------------------------
> 1 files changed, 42 insertions(+), 71 deletions(-)
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 8be18d4..25f8d41 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -2238,7 +2238,6 @@ error:
> int
> virDomainSave(virDomainPtr domain, const char *to)
> {
> - char filepath[4096];
> virConnectPtr conn;
>
> VIR_DOMAIN_DEBUG(domain, "to=%s", to);
> @@ -2260,29 +2259,24 @@ virDomainSave(virDomainPtr domain, const char *to)
> goto error;
> }
>
> - /*
> - * We must absolutize the file path as the save is done out of process
> - * TODO: check for URI when libxml2 is linked in.
> - */
> - if (to[0] != '/') {
> - unsigned int len, t;
> + if (conn->driver->domainSave) {
> + int ret;
> + char *absolute_to;
>
> - t = strlen(to);
> - if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL)
> - return -1;
> - len = strlen(filepath);
> - /* that should be covered by getcwd() semantic, but be 100% sure */
> - if (len > sizeof(filepath) - (t + 3))
> - return -1;
> - filepath[len] = '/';
> - strcpy(&filepath[len + 1], to);
> - to = &filepath[0];
> + /*
> + * We must absolutize the file path as the save is done out of process
> + * TODO: check for URI when libxml2 is linked in.
> + */
> + if (virFileAbsPath(to, &absolute_to) < 0) {
> + virLibConnError(VIR_ERR_INTERNAL_ERROR,
> + _("could not build absolute output file path"));
> + goto error;
> + }
>
> - }
> + ret = conn->driver->domainSave(domain, absolute_to);
> +
> + VIR_FREE(absolute_to);
>
> - if (conn->driver->domainSave) {
> - int ret;
> - ret = conn->driver->domainSave (domain, to);
> if (ret < 0)
> goto error;
> return ret;
> @@ -2298,7 +2292,7 @@ error:
> /**
> * virDomainRestore:
> * @conn: pointer to the hypervisor connection
> - * @from: path to the
> + * @from: path to the input file
> *
> * This method will restore a domain saved to disk by virDomainSave().
> *
> @@ -2307,7 +2301,6 @@ error:
> int
> virDomainRestore(virConnectPtr conn, const char *from)
> {
> - char filepath[4096];
> VIR_DEBUG("conn=%p, from=%s", conn, from);
>
> virResetLastError();
> @@ -2326,34 +2319,24 @@ virDomainRestore(virConnectPtr conn, const char *from)
> goto error;
> }
>
> - /*
> - * We must absolutize the file path as the restore is done out of process
> - * TODO: check for URI when libxml2 is linked in.
> - */
> - if (from[0] != '/') {
> - unsigned int len, t;
> + if (conn->driver->domainRestore) {
> + int ret;
> + char *absolute_from;
>
> - t = strlen(from);
> - if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL) {
> - virLibConnError(VIR_ERR_SYSTEM_ERROR,
> - _("cannot get working directory"));
> - goto error;
> - }
> - len = strlen(filepath);
> - /* that should be covered by getcwd() semantic, but be 100% sure */
> - if (len > sizeof(filepath) - (t + 3)) {
> + /*
> + * We must absolutize the file path as the restore is done out of process
> + * TODO: check for URI when libxml2 is linked in.
> + */
> + if (virFileAbsPath(from, &absolute_from) < 0) {
> virLibConnError(VIR_ERR_INTERNAL_ERROR,
> - _("path too long"));
> + _("could not build absolute input file path"));
> goto error;
> }
> - filepath[len] = '/';
> - strcpy(&filepath[len + 1], from);
> - from = &filepath[0];
> - }
>
> - if (conn->driver->domainRestore) {
> - int ret;
> - ret = conn->driver->domainRestore (conn, from);
> + ret = conn->driver->domainRestore(conn, absolute_from);
> +
> + VIR_FREE(absolute_from);
> +
> if (ret < 0)
> goto error;
> return ret;
> @@ -2381,7 +2364,6 @@ error:
> int
> virDomainCoreDump(virDomainPtr domain, const char *to, int flags)
> {
> - char filepath[4096];
> virConnectPtr conn;
>
> VIR_DOMAIN_DEBUG(domain, "to=%s, flags=%d", to, flags);
> @@ -2403,35 +2385,24 @@ virDomainCoreDump(virDomainPtr domain, const char *to, int flags)
> goto error;
> }
>
> - /*
> - * We must absolutize the file path as the save is done out of process
> - * TODO: check for URI when libxml2 is linked in.
> - */
> - if (to[0] != '/') {
> - unsigned int len, t;
> + if (conn->driver->domainCoreDump) {
> + int ret;
> + char *absolute_to;
>
> - t = strlen(to);
> - if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL) {
> - virLibDomainError(VIR_ERR_SYSTEM_ERROR,
> - _("cannot get current directory"));
> - goto error;
> - }
> - len = strlen(filepath);
> - /* that should be covered by getcwd() semantic, but be 100% sure */
> - if (len > sizeof(filepath) - (t + 3)) {
> - virLibDomainError(VIR_ERR_INTERNAL_ERROR,
> - _("path too long"));
> + /*
> + * We must absolutize the file path as the save is done out of process
> + * TODO: check for URI when libxml2 is linked in.
> + */
> + if (virFileAbsPath(to, &absolute_to) < 0) {
> + virLibConnError(VIR_ERR_INTERNAL_ERROR,
> + _("could not build absolute core file path"));
> goto error;
> }
> - filepath[len] = '/';
> - strcpy(&filepath[len + 1], to);
> - to = &filepath[0];
>
> - }
> + ret = conn->driver->domainCoreDump(domain, absolute_to, flags);
> +
> + VIR_FREE(absolute_to);
>
> - if (conn->driver->domainCoreDump) {
> - int ret;
> - ret = conn->driver->domainCoreDump (domain, to, flags);
> if (ret < 0)
> goto error;
> return ret;
ACK, but you could kill those TODO: comments, since I don't think we
will ever let libvirt save to arbitrary URIs. If anything we would
introduce a different API using the virStreamPtr facility, so the
client app would deal with URIs as needed
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list