[PATCH] gdiplus: Don't return copies of font families from GdipGetFontCollectionFamilyList.

[Nikolay Sivov]

On 1/24/19 1:59 PM, Sven Baars wrote:
> Signed-off-by: Sven Baars <sven.wine at gmail.com>
> ---
> This was added in 5694825ae3c3a581976716a62e1b2d1fb54e5674, but the
> corrected test shows this may not be correct. Maybe Vincent still knows
> why this was done?
>
>   dlls/gdiplus/font.c       | 20 ++++----------------
>   dlls/gdiplus/tests/font.c | 14 +++++++++-----
>   2 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c
> index 5e6aa5430f..cbe7859fe7 100644
> --- a/dlls/gdiplus/font.c
> +++ b/dlls/gdiplus/font.c
> @@ -1576,7 +1576,6 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList(
>           GpFontFamily* gpfamilies[], INT* numFound)
>   {
>       INT i;
> -    GpStatus stat=Ok;
>   
>       TRACE("%p, %d, %p, %p\n", fontCollection, numSought, gpfamilies, numFound);
>   
> @@ -1585,24 +1584,13 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList(
>   
>       memset(gpfamilies, 0, sizeof(*gpfamilies) * numSought);
>   
> -    for (i = 0; i < numSought && i < fontCollection->count && stat == Ok; i++)
> +    for (i = 0; i < numSought && i < fontCollection->count; i++)
>       {
> -        stat = GdipCloneFontFamily(fontCollection->FontFamilies[i], &gpfamilies[i]);
> +        gpfamilies[i] = fontCollection->FontFamilies[i];
>       }
> +    *numFound = i;
>   
> -    if (stat == Ok)
> -        *numFound = i;
> -    else
> -    {
> -        int numToFree=i;
> -        for (i=0; i<numToFree; i++)
> -        {
> -            GdipDeleteFontFamily(gpfamilies[i]);
> -            gpfamilies[i] = NULL;
> -        }
> -    }
> -
> -    return stat;
> +    return Ok;
>   }

Last time I looked at this one, I remember that it needed to be 
reference-counted.

Test path for that is to compare instance returned from GdipGetFamily() 
to what you get from collection.

>   
>   void free_installed_fonts(void)
> diff --git a/dlls/gdiplus/tests/font.c b/dlls/gdiplus/tests/font.c
> index 03e7a04e08..f91418d806 100644
> --- a/dlls/gdiplus/tests/font.c
> +++ b/dlls/gdiplus/tests/font.c
> @@ -1187,7 +1187,7 @@ todo_wine
>   
>   static void test_GdipGetFontCollectionFamilyList(void)
>   {
> -    GpFontFamily *family, *family2;
> +    GpFontFamily *family, *family2, **families;
>       GpFontCollection *collection;
>       INT found, count;
>       GpStatus status;
> @@ -1228,15 +1228,19 @@ static void test_GdipGetFontCollectionFamilyList(void)
>       ok(found == 1, "Unexpected list count %d.\n", found);
>       ok(family != NULL, "Expected family instance.\n");
>   
> -    family = NULL;
> +    family2 = NULL;
>       found = 0;
>       status = GdipGetFontCollectionFamilyList(collection, 1, &family2, &found);
>       ok(status == Ok, "Failed to get family list, status %d.\n", status);
>       ok(found == 1, "Unexpected list count %d.\n", found);
> -    ok(family2 != family, "Unexpected family instance.\n");
> +    ok(family2 == family, "Unexpected family instance.\n");
>   
> -    GdipDeleteFontFamily(family);
> -    GdipDeleteFontFamily(family2);
> +    families = GdipAlloc((count + 1) * sizeof(*families));
> +    found = 0;
> +    status = GdipGetFontCollectionFamilyList(collection, count + 1, families, &found);
> +    ok(status == Ok, "Failed to get family list, status %d.\n", status);
> +    ok(found == count, "Unexpected list count %d, extected %d.\n", found, count);
> +    GdipFree(families);
>   }
>   
>   static void test_GdipGetFontCollectionFamilyCount(void)