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

[James Hawkins]

On Thu, Jun 12, 2008 at 3:39 AM, Nikolay Sivov  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.