[PATCH v2] dwrite: Do not segfault when there are no typographic features to return.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
---
 dlls/dwrite/shape.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/dlls/dwrite/shape.c b/dlls/dwrite/shape.c
index 6428bf6ceb0..809e66ebedb 100644
--- a/dlls/dwrite/shape.c
+++ b/dlls/dwrite/shape.c
@@ -372,6 +372,14 @@ HRESULT shape_get_typographic_features(struct scriptshaping_context *context, co
     shape_get_script_lang_index(context, scripts, MS_GPOS_TAG, &script_index, &language_index);
     opentype_get_typographic_features(&context->cache->gpos, script_index, language_index, &t);
 
+    /* Return immediately if there are not features, because the
+       duplicate removal algorithm doesn't like an empty array. */
+    if (t.count == 0)
+    {
+        heap_free(t.tags);
+        return S_OK;
+    }
+
     /* Sort and remove duplicates. */
     qsort(t.tags, t.count, sizeof(*t.tags), tag_array_sorting_compare);
 
-- 
2.30.1

[Nikolay Sivov]

On 3/3/21 4:35 PM, Giovanni Mascellani wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
> ---
>  dlls/dwrite/shape.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/dlls/dwrite/shape.c b/dlls/dwrite/shape.c
> index 6428bf6ceb0..809e66ebedb 100644
> --- a/dlls/dwrite/shape.c
> +++ b/dlls/dwrite/shape.c
> @@ -372,6 +372,14 @@ HRESULT shape_get_typographic_features(struct scriptshaping_context *context, co
>      shape_get_script_lang_index(context, scripts, MS_GPOS_TAG, &script_index, &language_index);
>      opentype_get_typographic_features(&context->cache->gpos, script_index, language_index, &t);
>
> +    /* Return immediately if there are not features, because the
> +       duplicate removal algorithm doesn't like an empty array. */
> +    if (t.count == 0)
> +    {
> +        heap_free(t.tags);
> +        return S_OK;
> +    }
> +
>      /* Sort and remove duplicates. */
>      qsort(t.tags, t.count, sizeof(*t.tags), tag_array_sorting_compare);

Out argument is not set; 'tags' would be null if count is 0; comment is
unnecessary, it's pretty obvious what's going on.

[Giovanni Mascellani]

Il 03/03/21 14:42, Nikolay Sivov ha scritto:
> 'tags' would be null if count is 0

My philosophy here is that tags being NULL is just an artifact of 
implementation; if the code working with that struct changes, it might 
happen that count is zero, but tags is not NULL, so in the future it 
might happen that changes in another function makes this one buggy, 
which would be nice to avoid. To me, if an object is initialized, then 
it should be properly destructed, without assuming which code path it 
will have gone though by the time it is destructed.

However, no problem removing that line. Ack for the rest, will submit 
another version.

Giovanni.