qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 2/6] block: vmdk - move string allocations fr


From: John Snow
Subject: Re: [Qemu-devel] [PATCH v2 2/6] block: vmdk - move string allocations from stack to the heap
Date: Tue, 20 Jan 2015 13:37:07 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0



On 01/20/2015 12:31 PM, Jeff Cody wrote:
Functions 'vmdk_parse_extents' and 'vmdk_create' allocate several
PATH_MAX sized arrays on the stack.  Make these dynamically allocated.

Signed-off-by: Jeff Cody <address@hidden>
---
  block/vmdk.c | 64 ++++++++++++++++++++++++++++++++++++------------------------
  1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index dc6459c..043a750 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -784,7 +784,7 @@ static int vmdk_open_sparse(BlockDriverState *bs,
  static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
                                const char *desc_file_path, Error **errp)
  {
-    int ret;
+    int ret = 0;
      int matches;
      char access[11];
      char type[11];
@@ -792,12 +792,11 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
      const char *p = desc;
      int64_t sectors = 0;
      int64_t flat_offset;
-    char extent_path[PATH_MAX];
+    char *extent_path = g_malloc0(PATH_MAX);
      BlockDriverState *extent_file;
      BDRVVmdkState *s = bs->opaque;
      VmdkExtent *extent;

-

The stray newline you added is now gone!

      while (*p) {
          /* parse extent line in one of below formats:
           *
@@ -814,18 +813,21 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
          } else if (!strcmp(type, "FLAT")) {
              if (matches != 5 || flat_offset < 0) {
                  error_setg(errp, "Invalid extent lines: \n%s", p);
-                return -EINVAL;
+                ret = -EINVAL;
+                goto exit;
              }
          } else if (!strcmp(type, "VMFS")) {
              if (matches == 4) {
                  flat_offset = 0;
              } else {
                  error_setg(errp, "Invalid extent lines:\n%s", p);
-                return -EINVAL;
+                ret = -EINVAL;
+                goto exit;
              }
          } else if (matches != 4) {
              error_setg(errp, "Invalid extent lines:\n%s", p);
-            return -EINVAL;
+            ret = -EINVAL;
+            goto exit;
          }

          if (sectors <= 0 ||
@@ -840,7 +842,8 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
          {
              error_setg(errp, "Cannot use relative extent paths with VMDK "
                         "descriptor file '%s'", bs->file->filename);
-            return -EINVAL;
+            ret = -EINVAL;
+            goto exit;
          }

          path_combine(extent_path, sizeof(extent_path),
@@ -849,7 +852,7 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
          ret = bdrv_open(&extent_file, extent_path, NULL, NULL,
                          bs->open_flags | BDRV_O_PROTOCOL, NULL, errp);
          if (ret) {
-            return ret;
+            goto exit;
          }

          /* save to extents array */
@@ -860,7 +863,7 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
                              0, 0, 0, 0, 0, &extent, errp);
              if (ret < 0) {
                  bdrv_unref(extent_file);
-                return ret;
+                goto exit;
              }
              extent->flat_start_offset = flat_offset << 9;
          } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
@@ -874,13 +877,14 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
              g_free(buf);
              if (ret) {
                  bdrv_unref(extent_file);
-                return ret;
+                goto exit;
              }
              extent = &s->extents[s->num_extents - 1];
          } else {
              error_setg(errp, "Unsupported extent type '%s'", type);
              bdrv_unref(extent_file);
-            return -ENOTSUP;
+            ret = -ENOTSUP;
+            goto exit;
          }
          extent->type = g_strdup(type);
  next_line:
@@ -893,7 +897,9 @@ next_line:
              p++;
          }
      }
-    return 0;
+exit:
+    g_free(extent_path);
+    return ret;
  }

  static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
@@ -1797,10 +1803,15 @@ static int vmdk_create(const char *filename, QemuOpts 
*opts, Error **errp)
      int ret = 0;
      bool flat, split, compress;
      GString *ext_desc_lines;
-    char path[PATH_MAX], prefix[PATH_MAX], postfix[PATH_MAX];
+    char *path = g_malloc0(PATH_MAX);
+    char *prefix = g_malloc0(PATH_MAX);
+    char *postfix = g_malloc0(PATH_MAX);
+    char *desc_line = g_malloc0(BUF_SIZE);
+    char *ext_filename = g_malloc0(PATH_MAX);
+    char *desc_filename = g_malloc0(PATH_MAX);
      const int64_t split_size = 0x80000000;  /* VMDK has constant split size */
      const char *desc_extent_line;
-    char parent_desc_line[BUF_SIZE] = "";
+    char *parent_desc_line = g_malloc0(BUF_SIZE);
      uint32_t parent_cid = 0xffffffff;
      uint32_t number_heads = 16;
      bool zeroed_grain = false;
@@ -1916,33 +1927,27 @@ static int vmdk_create(const char *filename, QemuOpts 
*opts, Error **errp)
          }
          parent_cid = vmdk_read_cid(bs, 0);
          bdrv_unref(bs);
-        snprintf(parent_desc_line, sizeof(parent_desc_line),
+        snprintf(parent_desc_line, BUF_SIZE,
                  "parentFileNameHint=\"%s\"", backing_file);
      }

      /* Create extents */
      filesize = total_size;
      while (filesize > 0) {
-        char desc_line[BUF_SIZE];
-        char ext_filename[PATH_MAX];
-        char desc_filename[PATH_MAX];
          int64_t size = filesize;

          if (split && size > split_size) {
              size = split_size;
          }
          if (split) {
-            snprintf(desc_filename, sizeof(desc_filename), "%s-%c%03d%s",
+            snprintf(desc_filename, PATH_MAX, "%s-%c%03d%s",
                      prefix, flat ? 'f' : 's', ++idx, postfix);
          } else if (flat) {
-            snprintf(desc_filename, sizeof(desc_filename), "%s-flat%s",
-                    prefix, postfix);
+            snprintf(desc_filename, PATH_MAX, "%s-flat%s", prefix, postfix);
          } else {
-            snprintf(desc_filename, sizeof(desc_filename), "%s%s",
-                    prefix, postfix);
+            snprintf(desc_filename, PATH_MAX, "%s%s", prefix, postfix);
          }
-        snprintf(ext_filename, sizeof(ext_filename), "%s%s",
-                path, desc_filename);
+        snprintf(ext_filename, PATH_MAX, "%s%s", path, desc_filename);

          if (vmdk_create_extent(ext_filename, size,
                                 flat, compress, zeroed_grain, opts, errp)) {
@@ -1952,7 +1957,7 @@ static int vmdk_create(const char *filename, QemuOpts 
*opts, Error **errp)
          filesize -= size;

          /* Format description line */
-        snprintf(desc_line, sizeof(desc_line),
+        snprintf(desc_line, BUF_SIZE,
                      desc_extent_line, size / BDRV_SECTOR_SIZE, desc_filename);
          g_string_append(ext_desc_lines, desc_line);
      }
@@ -2007,6 +2012,13 @@ exit:
      g_free(backing_file);
      g_free(fmt);
      g_free(desc);
+    g_free(path);
+    g_free(prefix);
+    g_free(postfix);
+    g_free(desc_line);
+    g_free(ext_filename);
+    g_free(desc_filename);
+    g_free(parent_desc_line);
      g_string_free(ext_desc_lines, true);
      return ret;
  }


Reviewed-by: John Snow <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]