[PATCH 0/1] MR2281: nsi: Simplify and better document NsiAllocateAndGetTable().

[PATCH 1/1] nsi: Simplify and better document NsiAllocateAndGetTable().

[Francois Gouget]

From: Francois Gouget

Return directly in case of success so exiting the loop only happens in
case of error.
Better identify which statements prepare the next attempt / set things
up in case the table allocation failed.
---
 dlls/nsi/nsi.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/dlls/nsi/nsi.c b/dlls/nsi/nsi.c
index 3f324ef555b..52bbe4131a4 100644
--- a/dlls/nsi/nsi.c
+++ b/dlls/nsi/nsi.c
@@ -39,9 +39,9 @@ DWORD WINAPI NsiAllocateAndGetTable( DWORD unk, const NPI_MODULEID *module, DWOR
                                      void **rw_data, DWORD rw_size, void **dynamic_data, DWORD dynamic_size,
                                      void **static_data, DWORD static_size, DWORD *count, DWORD unk2 )
 {
-    DWORD err, num = 64;
     void *data[4] = { NULL };
     DWORD sizes[4] = { key_size, rw_size, dynamic_size, static_size };
+    DWORD err = ERROR_OUTOFMEMORY, num = 64;
     int i, attempt;

TRACE( "%ld %p %ld %p %ld %p %ld %p %ld %p %ld %p %ld\n", unk, module, table, key_data, key_size,
@@ -54,37 +54,35 @@ DWORD WINAPI NsiAllocateAndGetTable( DWORD unk, const NPI_MODULEID *module, DWOR
             if (sizes[i])
             {
                 data[i] = heap_alloc( sizes[i] * num );
-                if (!data[i])
-                {
-                    err = ERROR_OUTOFMEMORY;
-                    goto err;
-                }
+                if (!data[i]) goto err;
             }
         }

err = NsiEnumerateObjectsAllParameters( unk, 0, module, table, data[0], sizes[0], data[1], sizes[1],
                                                 data[2], sizes[2], data[3], sizes[3], &num );
+        if (err == NO_ERROR)
+        {
+            if (sizes[0]) *key_data = data[0];
+            if (sizes[1]) *rw_data = data[1];
+            if (sizes[2]) *dynamic_data = data[2];
+            if (sizes[3]) *static_data = data[3];
+            *count = num;
+            return NO_ERROR;
+        }
         if (err != ERROR_MORE_DATA) break;
         TRACE( "Short buffer, attempt %d.\n", attempt );
-        NsiFreeTable( data[0], data[1], data[2], data[3] );
-        memset( data, 0, sizeof(data) );
+        /* num is not updated in the ERROR_MORE_DATA case :-( */
         err = NsiEnumerateObjectsAllParameters( unk, 0, module, table, NULL, 0, NULL, 0, NULL, 0, NULL, 0, &num );
-        if (err) return err;
-        err = ERROR_OUTOFMEMORY; /* fail if this is the last attempt */
+        if (err) break;
         num += num >> 4; /* the tables may grow before the next iteration; get ahead */
-    }
-
-    if (!err)
-    {
-        if (sizes[0]) *key_data = data[0];
-        if (sizes[1]) *rw_data = data[1];
-        if (sizes[2]) *dynamic_data = data[2];
-        if (sizes[3]) *static_data = data[3];
-        *count = num;
+        /* prepare next attempt or fail if this was the last */
+        NsiFreeTable( data[0], data[1], data[2], data[3] );
+        memset( data, 0, sizeof(data) );
+        err = ERROR_OUTOFMEMORY;
     }

err:
-    if (err) NsiFreeTable( data[0], data[1], data[2], data[3] );
+    NsiFreeTable( data[0], data[1], data[2], data[3] );
     return err;
 }

[Huw Davies (＠huw)]

I don't find this significantly clearer to make this change worthwhile.

[PATCH 0/1] MR2281: nsi: Simplify and better document NsiAllocateAndGetTable(). - closed

[Huw Davies (＠huw)]

This merge request was closed by Huw Davies.