[PATCH 0/1] MR7099: windowscodecs: Simplify png_decoder_get_metadata_blocks using realloc.

[Jeffrey Smith (＠whydoubt)]

The code currently `malloc`s a larger memory buffer, and if successful: `memcpy`s the old memory buffer to the new, `free`s the old, and reassigns the pointer.  This logic can all be reduced to a `realloc`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7099

[PATCH 1/1] windowscodecs: Simplify png_decoder_get_metadata_blocks using realloc.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/windowscodecs/libpng.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/dlls/windowscodecs/libpng.c b/dlls/windowscodecs/libpng.c
index 137a8e15e33..73eaa1830a0 100644
--- a/dlls/windowscodecs/libpng.c
+++ b/dlls/windowscodecs/libpng.c
@@ -377,10 +377,12 @@ static HRESULT CDECL png_decoder_get_metadata_blocks(struct decoder* iface,
     do
     {
         hr = stream_seek(This->stream, seek, STREAM_SEEK_SET, &chunk_start);
-        if (FAILED(hr)) goto end;
+        if (FAILED(hr))
+            break;
 
         hr = read_png_chunk(This->stream, chunk_type, NULL, &chunk_size);
-        if (FAILED(hr)) goto end;
+        if (FAILED(hr))
+            break;
 
         if (chunk_type[0] >= 'a' && chunk_type[0] <= 'z' &&
             memcmp(chunk_type, "tRNS", 4) && memcmp(chunk_type, "pHYs", 4))
@@ -388,24 +390,14 @@ static HRESULT CDECL png_decoder_get_metadata_blocks(struct decoder* iface,
             /* This chunk is considered metadata. */
             if (*count == metadata_blocks_size)
             {
-                struct decoder_block *new_metadata_blocks;
-                ULONG new_metadata_blocks_size;
-
-                new_metadata_blocks_size = 4 + metadata_blocks_size * 2;
-                new_metadata_blocks = malloc(new_metadata_blocks_size * sizeof(*new_metadata_blocks));
+                metadata_blocks_size = 4 + metadata_blocks_size * 2;
+                result = realloc(result, metadata_blocks_size * sizeof(*result));
 
-                if (!new_metadata_blocks)
+                if (!result)
                 {
                     hr = E_OUTOFMEMORY;
-                    goto end;
+                    break;
                 }
-
-                memcpy(new_metadata_blocks, result,
-                    *count * sizeof(*new_metadata_blocks));
-
-                free(result);
-                result = new_metadata_blocks;
-                metadata_blocks_size = new_metadata_blocks_size;
             }
 
             result[*count].offset = chunk_start;
@@ -417,7 +409,6 @@ static HRESULT CDECL png_decoder_get_metadata_blocks(struct decoder* iface,
         seek = chunk_start + chunk_size + 12; /* skip data and CRC */
     } while (memcmp(chunk_type, "IEND", 4));
 
-end:
     if (SUCCEEDED(hr))
     {
         *blocks = result;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7099

[Alfred Agrell (＠Alcaro)]

Alfred Agrell (@Alcaro) commented about dlls/windowscodecs/libpng.c:
> /* This chunk is considered metadata. */
>              if (*count == metadata_blocks_size)
>              {
> -                struct decoder_block *new_metadata_blocks;
> -                ULONG new_metadata_blocks_size;
> -
> -                new_metadata_blocks_size = 4 + metadata_blocks_size * 2;
> -                new_metadata_blocks = malloc(new_metadata_blocks_size * sizeof(*new_metadata_blocks));
> +                metadata_blocks_size = 4 + metadata_blocks_size * 2;
> +                result = realloc(result, metadata_blocks_size * sizeof(*result));

If realloc fails, the original memory area remains unchanged. x = realloc(x, y) loses the original pointer, yielding a memory leak.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7099#note_91344