[PATCH 0/1] MR3089: hhctrl: Simplify allocation size in parse_index_sitemap_object (scan-build).

[Alex Henrie (＠alexhenrie)]

sizeof(IndexSubItem) is more clear than 2 * sizeof(void *) and it avoids
any questions about struct padding.

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

[PATCH 1/1] hhctrl: Simplify allocation size in parse_index_sitemap_object (scan-build).

[Alex Henrie]

From: Alex Henrie <alexhenrie24(a)gmail.com>

sizeof(IndexSubItem) is more clear than 2 * sizeof(void *) and it avoids
any questions about struct padding.
---
 dlls/hhctrl.ocx/index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/hhctrl.ocx/index.c b/dlls/hhctrl.ocx/index.c
index 113daa9efc9..97781fc10a9 100644
--- a/dlls/hhctrl.ocx/index.c
+++ b/dlls/hhctrl.ocx/index.c
@@ -129,7 +129,7 @@ static IndexItem *parse_index_sitemap_object(HHInfo *info, stream_t *stream)
 
     item = calloc(1, sizeof(IndexItem));
     item->nItems = 0;
-    item->items = calloc(2, sizeof(void *));
+    item->items = calloc(1, sizeof(IndexSubItem));
     item->itemFlags = 0x11;
 
     while(next_node(stream, &node)) {
-- 
GitLab

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

[Erich Hoover (＠ehoover)]

It doesn't matter what you allocate here, the pointer will be replaced when `item_realloc()` gets called.  This line was originally an allocation of size 0 that got changed in commit 9dba420d0a78:
```
-    item = heap_alloc_zero(sizeof(IndexItem));
+    item = calloc(1, sizeof(IndexItem));
     item->nItems = 0;
-    item->items = heap_alloc_zero(0);
+    item->items = calloc(2, sizeof(void *));
     item->itemFlags = 0x11;
```
So, I would suggest making this a `malloc(0)` instead so that it's clear that we just want the pointer for later use in `realloc`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3089#note_36033

[Alex Henrie (＠alexhenrie)]

On Sun Jun 18 00:44:41 2023 +0000, Erich Hoover wrote:
> It doesn't matter what you allocate here, the pointer will be replaced
> when `item_realloc()` gets called.  This line was originally an
> allocation of size 0 that got changed in commit 9dba420d0a78:
> ```
> -    item = heap_alloc_zero(sizeof(IndexItem));
> +    item = calloc(1, sizeof(IndexItem));
>      item->nItems = 0;
> -    item->items = heap_alloc_zero(0);
> +    item->items = calloc(2, sizeof(void *));
>      item->itemFlags = 0x11;
> ```
> So, I would suggest making this a `malloc(0)` instead so that it's clear
> that we just want the pointer for later use in `realloc`.
> I remember being confused as to the intention here when I converted this library to use CRT allocation functions. If the initial value is only used as a parameter to realloc, let's just initialize it to NULL.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3089#note_36037

[Erich Hoover (＠ehoover)]

On Sun Jun 18 00:44:41 2023 +0000, Alex Henrie wrote:
> I remember being confused as to the intention here when I converted this
> library to use CRT allocation functions. If the initial value is only
> used as a parameter to realloc, let's just initialize it to NULL.
> Wine might not care, but that won't work on all platforms (I have been burned by that before).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3089#note_36038

[PATCH 0/1] MR3089: hhctrl: Initialize item->items to NULL in parse_index_sitemap_object (scan-build).

[Alex Henrie (＠alexhenrie)]

On Sun Jun 18 00:46:24 2023 +0000, Erich Hoover wrote:
> Wine might not care, but that won't work on all platforms (I have been
> burned by that before).
> I'm afraid that `malloc(0)` is even less likely to work in all CRT implementations. `realloc(NULL, size)` where size > 0 at least has the advantage of being required to work according to all versions of the C standard. In any case, it definitely works in Wine's msvcrt.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3089#note_36039