[libvirt] [PATCH 09/20] Use virFileAbsPath instead of manually creating the absolute path

[Daniel P. Berrange]

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 :|