[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH 5/5] d3dx9: Correctly handle out of memory conditions while reallocating children array.
From:       Matteo Bruni <matteo.mystral () gmail ! com>
Date:       2014-08-26 16:07:17
Message-ID: CABvNrtNF-xnY+WaCkxEVJHGC+cwFxMDLucVFdpxfSShpLFekEA () mail ! gmail ! com
[Download RAW message or body]

2014-08-26 16:58 GMT+02:00 Matteo Bruni <matteo.mystral@gmail.com>:
> 2014-08-26 16:30 GMT+02:00 Henri Verbeet <hverbeet@gmail.com>:
> > On 26 August 2014 16:13, Matteo Bruni <mbruni@codeweavers.com> wrote:
> > > @@ -338,15 +338,20 @@ static HRESULT d3dx9_file_data_create(IDirectXFileObject \
> > > *dxfile_object, ID3DXFi 
> > > while (SUCCEEDED(ret = IDirectXFileData_GetNextObject(object->dxfile_data, \
> > > &data_object)))
> > ...
> > > +            new_children = HeapReAlloc(GetProcessHeap(), 0, object->children,
> > > +                    sizeof(*object->children) * (object->nb_children + 1));
> > While I suspect this is hardly the only thing wrong with xfile.c, it
> > seems suboptimal to grow the array by 1 each time you go through the
> > loop.
> 
> Yeah, it should start at some size greater than 1 (no idea about
> typical children count here though) and then grow exponentially. I
> guess that can be a separate patch, but I can also add it to this one
> and resend if preferred.

Actually, merging the two results in a bit less churn.
So, this patch replaces the original one. I manually generated this
patch via "git diff" (and kept the individual commits locally just in
case), if this doesn't work right you can either commit the original
one as-is or just leave it out.


["merged.txt" (text/plain)]

diff --git a/dlls/d3dx9_36/xfile.c b/dlls/d3dx9_36/xfile.c
index f5f3bae..7386ec9 100644
--- a/dlls/d3dx9_36/xfile.c
+++ b/dlls/d3dx9_36/xfile.c
@@ -298,6 +298,7 @@ static HRESULT d3dx9_file_data_create(IDirectXFileObject \
*dxfile_object, ID3DXFi  {
     struct d3dx9_file_data *object;
     IDirectXFileObject *data_object;
+    unsigned int children_array_size = 0;
     HRESULT ret;
 
     TRACE("dxfile_object %p, ret_iface %p.\n", dxfile_object, ret_iface);
@@ -338,14 +339,27 @@ static HRESULT d3dx9_file_data_create(IDirectXFileObject \
*dxfile_object, ID3DXFi  
     while (SUCCEEDED(ret = IDirectXFileData_GetNextObject(object->dxfile_data, \
&data_object)))  {
-        if (object->children)
-            object->children = HeapReAlloc(GetProcessHeap(), 0, object->children, \
                sizeof(ID3DXFileData*) * (object->nb_children + 1));
-        else
-            object->children = HeapAlloc(GetProcessHeap(), 0, \
                sizeof(ID3DXFileData*));
-        if (!object->children)
+        if (object->nb_children >= children_array_size)
         {
-            ret = E_OUTOFMEMORY;
-            break;
+            ID3DXFileData **new_children;
+
+            if (object->children)
+            {
+                children_array_size *= 2;
+                new_children = HeapReAlloc(GetProcessHeap(), 0, object->children,
+                        sizeof(*object->children) * children_array_size);
+            }
+            else
+            {
+                children_array_size = 4;
+                new_children = HeapAlloc(GetProcessHeap(), 0, \
sizeof(*object->children) * children_array_size); +            }
+            if (!new_children)
+            {
+                ret = E_OUTOFMEMORY;
+                break;
+            }
+            object->children = new_children;
         }
         ret = d3dx9_file_data_create(data_object, \
&object->children[object->nb_children]);  IUnknown_Release(data_object);
@@ -353,12 +367,20 @@ static HRESULT d3dx9_file_data_create(IDirectXFileObject \
*dxfile_object, ID3DXFi  break;
         object->nb_children++;
     }
-
     if (ret != DXFILEERR_NOMOREOBJECTS)
     {
         (&object->ID3DXFileData_iface)->lpVtbl->Release(&object->ID3DXFileData_iface);
  return ret;
     }
+    if (object->children)
+    {
+        ID3DXFileData **new_children;
+
+        new_children = HeapReAlloc(GetProcessHeap(), 0, object->children,
+                sizeof(*object->children) * object->nb_children);
+        if (new_children)
+            object->children = new_children;
+    }
 
     TRACE("Found %u children\n", object->nb_children);
 
@@ -537,6 +559,7 @@ static HRESULT WINAPI d3dx9_file_CreateEnumObject(ID3DXFile \
*iface, const void *  DXFILELOADRESOURCE dxfile_resource;
     DXFILELOADMEMORY dxfile_memory;
     IDirectXFileData *data_object;
+    unsigned children_array_size = 0;
     HRESULT ret;
 
     TRACE("iface %p, source %p, options %#x, enum_object %p.\n", iface, source, \
options, enum_object); @@ -594,14 +617,27 @@ static HRESULT WINAPI \
d3dx9_file_CreateEnumObject(ID3DXFile *iface, const void *  /* Fill enum object with \
                top level data objects */
     while (SUCCEEDED(ret = \
IDirectXFileEnumObject_GetNextDataObject(dxfile_enum_object, &data_object)))  {
-        if (object->children)
-            object->children = HeapReAlloc(GetProcessHeap(), 0, object->children, \
                sizeof(*object->children) * (object->nb_children + 1));
-        else
-            object->children = HeapAlloc(GetProcessHeap(), 0, \
                sizeof(*object->children));
-        if (!object->children)
+        if (object->nb_children >= children_array_size)
         {
-            ret = E_OUTOFMEMORY;
-            break;
+            ID3DXFileData **new_children;
+
+            if (object->children)
+            {
+                children_array_size *= 2;
+                new_children = HeapReAlloc(GetProcessHeap(), 0, object->children,
+                        sizeof(*object->children) * children_array_size);
+            }
+            else
+            {
+                children_array_size = 4;
+                new_children = HeapAlloc(GetProcessHeap(), 0, \
sizeof(*object->children) * children_array_size); +            }
+            if (!new_children)
+            {
+                ret = E_OUTOFMEMORY;
+                break;
+            }
+            object->children = new_children;
         }
         ret = d3dx9_file_data_create((IDirectXFileObject*)data_object,
                 &object->children[object->nb_children]);
@@ -610,6 +646,15 @@ static HRESULT WINAPI d3dx9_file_CreateEnumObject(ID3DXFile \
*iface, const void *  break;
         object->nb_children++;
     }
+    if (object->children)
+    {
+        ID3DXFileData **new_children;
+
+        new_children = HeapReAlloc(GetProcessHeap(), 0, object->children,
+                sizeof(*object->children) * object->nb_children);
+        if (new_children)
+            object->children = new_children;
+    }
 
     IDirectXFileEnumObject_Release(dxfile_enum_object);
 





[prev in list] [next in list] [prev in thread] [next in thread]