[libvirt] [PATCH] Eliminate use of large buffer on stack in virFileMakePath.

[Daniel Veillard]

On Fri, Jan 22, 2010 at 02:04:19PM -0500, Laine Stump wrote:
> virFileMakePath is a recursive function that was creates a buffer
> PATH_MAX bytes long for each recursion (one recursion for each element
> in the path). This changes it to have no buffers on the stack, and to
> allocate just one buffer total, no matter how many elements are in the
> path. Because the modified algorithm requires a char* to be passed in
> rather than const char *, it is now 2 functions - a toplevel API
> function that remains identical in function, and a 2nd helper function
> called for the recursions, which 1) doesn't allocate anything, and 2)
> takes a char* arg, so it can modify the contents.
> 
> src/util/util.c: rewrite virFileMakePath
> ---
>  src/util/util.c |   58 +++++++++++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 46 insertions(+), 12 deletions(-)
> 
> diff --git a/src/util/util.c b/src/util/util.c
> index 08d74a3..0ce5026 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -1443,34 +1443,68 @@ int virDirCreate(const char *path, mode_t mode,
>  }
>  #endif
>  
> -int virFileMakePath(const char *path)
> -{
> +static int virFileMakePathHelper(char *path) {
>      struct stat st;
> -    char parent[PATH_MAX];
> -    char *p;
> +    char *p = NULL;
>      int err;
>  
>      if (stat(path, &st) >= 0)
>          return 0;
>  
> -    if (virStrcpyStatic(parent, path) == NULL)
> -        return EINVAL;
> -
> -    if (!(p = strrchr(parent, '/')))
> +    if ((p = strrchr(path, '/')) == NULL)
>          return EINVAL;
>  
> -    if (p != parent) {
> +    if (p != path) {
>          *p = '\0';
> -        if ((err = virFileMakePath(parent)))
> +        err = virFileMakePathHelper(path);
> +        *p = '/';
> +        if (err != 0)
>              return err;
>      }
>  
> -    if (mkdir(path, 0777) < 0 && errno != EEXIST)
> +    if (mkdir(path, 0777) < 0 && errno != EEXIST) {
>          return errno;
> -
> +    }
>      return 0;
>  }
>  
> +int virFileMakePath(const char *path)
> +{
> +    struct stat st;
> +    char *parent = NULL;
> +    char *p;
> +    int err = 0;
> +
> +    if (stat(path, &st) >= 0)
> +        goto cleanup;
> +
> +    if ((parent = strdup(path)) == NULL) {
> +        err = ENOMEM;
> +        goto cleanup;
> +    }
> +
> +    if ((p = strrchr(parent, '/')) == NULL) {
> +        err = EINVAL;
> +        goto cleanup;
> +    }
> +
> +    if (p != parent) {
> +        *p = '\0';
> +        if ((err = virFileMakePathHelper(parent)) != 0) {
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (mkdir(path, 0777) < 0 && errno != EEXIST) {
> +        err = errno;
> +        goto cleanup;
> +    }
> +
> +cleanup:
> +    VIR_FREE(parent);
> +    return err;
> +}
> +
>  /* Build up a fully qualfiied path for a config file to be
>   * associated with a persistent guest or network */
>  int virFileBuildPath(const char *dir,

  ACK, good catch !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/