[PATCH v2 0/1] MR1778: dwrite: Handle memory allocation failure in bidi_compute_bracket_pairs (cppcheck).

[Alex Henrie (＠alexhenrie)]

--
  v2: dwrite: Handle memory allocation failure in bidi_compute_bracket_pairs (cppcheck).

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

[PATCH v2 1/1] dwrite: Handle memory allocation failure in bidi_compute_bracket_pairs (cppcheck).

[Alex Henrie]

From: Alex Henrie

---
 dlls/dwrite/bidi.c | 65 +++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 33 deletions(-)

diff --git a/dlls/dwrite/bidi.c b/dlls/dwrite/bidi.c
index 142c82f19d0..940f147b880 100644
--- a/dlls/dwrite/bidi.c
+++ b/dlls/dwrite/bidi.c
@@ -632,51 +632,50 @@ static BracketPair *bidi_compute_bracket_pairs(IsolatedRun *iso_run)
     WCHAR *open_stack;
     int *stack_index;
     int stack_top = iso_run->length;
-    BracketPair *out = NULL;
+    BracketPair *out;
     int pair_count = 0;
     int i;

open_stack = malloc(sizeof(WCHAR) * iso_run->length);
     stack_index = malloc(sizeof(int) * iso_run->length);
+    out = malloc(sizeof(BracketPair) * iso_run->length);

-    for (i = 0; i < iso_run->length; i++) {
-        unsigned short ubv = get_table_entry_16(bidi_bracket_table, iso_run->item[i].ch);
-        if (ubv)
-        {
-            if (!out)
-            {
-                out = malloc(sizeof(BracketPair));
-                out[0].start = -1;
-            }
+    if (open_stack && stack_index && out) {
+        out[0].start = -1;

-            if ((ubv >> 8) == 0) {
-                stack_top--;
-                open_stack[stack_top] = iso_run->item[i].ch + (signed char)(ubv & 0xff);
-                /* deal with canonical equivalent U+2329/232A and U+3008/3009 */
-                if (open_stack[stack_top] == 0x232A)
-                    open_stack[stack_top] = 0x3009;
-                stack_index[stack_top] = i;
-            }
-            else if ((ubv >> 8) == 1) {
-                int j;
-
-                if (stack_top == iso_run->length) continue;
-                for (j = stack_top; j < iso_run->length; j++) {
-                    WCHAR c = iso_run->item[i].ch;
-                    if (c == 0x232A) c = 0x3009;
-                    if (c == open_stack[j]) {
-                        out[pair_count].start = stack_index[j];
-                        out[pair_count].end = i;
-                        pair_count++;
-                        out = realloc(out, sizeof(BracketPair) * (pair_count+1));
-                        out[pair_count].start = -1;
-                        stack_top = j+1;
-                        break;
+        for (i = 0; i < iso_run->length; i++) {
+            unsigned short ubv = get_table_entry_16(bidi_bracket_table, iso_run->item[i].ch);
+            if (ubv)
+            {
+                if ((ubv >> 8) == 0) {
+                    stack_top--;
+                    open_stack[stack_top] = iso_run->item[i].ch + (signed char)(ubv & 0xff);
+                    /* deal with canonical equivalent U+2329/232A and U+3008/3009 */
+                    if (open_stack[stack_top] == 0x232A)
+                        open_stack[stack_top] = 0x3009;
+                    stack_index[stack_top] = i;
+                }
+                else if ((ubv >> 8) == 1) {
+                    int j;
+
+                    if (stack_top == iso_run->length) continue;
+                    for (j = stack_top; j < iso_run->length; j++) {
+                        WCHAR c = iso_run->item[i].ch;
+                        if (c == 0x232A) c = 0x3009;
+                        if (c == open_stack[j]) {
+                            out[pair_count].start = stack_index[j];
+                            out[pair_count].end = i;
+                            pair_count++;
+                            out[pair_count].start = -1;
+                            stack_top = j+1;
+                            break;
+                        }
                     }
                 }
             }
         }
     }
+
     if (pair_count == 0)
     {
         free(out);

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/dwrite/bidi.c:
>  open_stack = malloc(sizeof(WCHAR) * iso_run->length);
>  stack_index = malloc(sizeof(int) * iso_run->length);
> - out = malloc(sizeof(BracketPair) * iso_run->length);
> - for (i = 0; i < iso_run->length; i++) {
> -
>    unsigned short ubv = get_table_entry_16(bidi_bracket_table, iso_run->item[i].ch);
> -
>    if (ubv)
> -
>    {
> -
>        if (!out)
> -
>        {
> -
>            out = malloc(sizeof(BracketPair));
> -
>            out[0].start = -1;
> -
>        }
> - if (open_stack && stack_index && out) {
> -
>    out[0].start = -1;

No need to change that much. You can check all 3 allocations at once, and return on failure.