Re: [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
>
> 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)