Re: gdi32: GetCharacterPlacement should accept NULL lpResults (fixes bug 13094)

[James Hawkins]

On Thu, Jun 12, 2008 at 3:39 AM, Nikolay Sivov <bunglehead(a)gmail.com> wrote:
> Changelog:
>    - Fixes GetCharacterPlacement with NULL lpResults argument (fix for bug 13094)
>
> ---
>  dlls/gdi32/font.c |   34 ++++++++++++++++++++++++----------
>  1 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c
> index 1bfd97a..34aabd1 100644
> --- a/dlls/gdi32/font.c
> +++ b/dlls/gdi32/font.c
> @@ -2709,27 +2709,35 @@ GetCharacterPlacementA(HDC hdc, LPCSTR lpString, INT uCount,
>     WCHAR *lpStringW;
>     INT uCountW;
>     GCP_RESULTSW resultsW;
> +    GCP_RESULTSW *presults = NULL;
>     DWORD ret;
>     UINT font_cp;
>
> TRACE("%s, %d, %d, 0x%08x\n",
>           debugstr_an(lpString, uCount), uCount, nMaxExtent, dwFlags);
>
> -    /* both structs are equal in size */
> -    memcpy(&resultsW, lpResults, sizeof(resultsW));
> +    if(lpResults){
> +        /* both structs are equal in size */
> +        memcpy(&resultsW, lpResults, sizeof(resultsW));
> +
> +        if(lpResults->lpOutString)
> +            resultsW.lpOutString = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR)*uCountW);
> +
> +        presults = &resultsW;
> +    }
>
> lpStringW = FONT_mbtowc(hdc, lpString, uCount, &uCountW, &font_cp);
> -    if(lpResults->lpOutString)
> -        resultsW.lpOutString = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR)*uCountW);
>
> -    ret = GetCharacterPlacementW(hdc, lpStringW, uCountW, nMaxExtent, &resultsW, dwFlags);
> +    ret = GetCharacterPlacementW(hdc, lpStringW, uCountW, nMaxExtent, presults, dwFlags);
>
> -    lpResults->nGlyphs = resultsW.nGlyphs;
> -    lpResults->nMaxFit = resultsW.nMaxFit;
> +    if(lpResults){
> +        lpResults->nGlyphs = resultsW.nGlyphs;
> +        lpResults->nMaxFit = resultsW.nMaxFit;
>
> -    if(lpResults->lpOutString) {
> -        WideCharToMultiByte(font_cp, 0, resultsW.lpOutString, uCountW,
> -                            lpResults->lpOutString, uCount, NULL, NULL );
> +        if(lpResults->lpOutString) {
> +            WideCharToMultiByte(font_cp, 0, resultsW.lpOutString, uCountW,
> +                                lpResults->lpOutString, uCount, NULL, NULL );
> +        }
>     }
>
> HeapFree(GetProcessHeap(), 0, lpStringW);
> @@ -2780,6 +2788,10 @@ GetCharacterPlacementW(
>            lpResults->lpGlyphs, lpResults->nGlyphs, lpResults->nMaxFit);
>
> if(dwFlags&(~GCP_REORDER))                 FIXME("flags 0x%08x ignored\n", dwFlags);
> +
> +    /* with NULL lpResults return GetTextExtentPoint32 result */
> +    if(!lpResults)  goto nullres;
> +
>     if(lpResults->lpClass)     FIXME("classes not implemented\n");
>     if (lpResults->lpCaretPos && (dwFlags & GCP_REORDER))
>         FIXME("Caret positions for complex scripts not implemented\n");
> @@ -2833,6 +2845,8 @@ GetCharacterPlacementW(
>     if(lpResults->lpGlyphs)
>        GetGlyphIndicesW(hdc, lpString, nSet, lpResults->lpGlyphs, 0);
>
> +nullres:
> +
>     if (GetTextExtentPoint32W(hdc, lpString, uCount, &size))
>       ret = MAKELONG(size.cx, size.cy);

You need to write a test for this behavior, especially this close to a release.

-- 
James Hawkins