Re: gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font associated charset

[Akihiro Sagawa]

On Sun, 24 Feb 2013 09:55:59 +0900, Byeongsik Jeon wrote:

Could you consider adding "(try N)" at the end of subject line?
The rule is documented here:
http://wiki.winehq.org/SubmittingPatches#head-fea51bfdf6a29105a9a6fc6f5886ad...
> diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c
> index b274b83..3a99d35 100644
> --- a/dlls/gdi32/font.c
> +++ b/dlls/gdi32/font.c
> @@ -590,11 +590,75 @@ HFONT WINAPI CreateFontW( INT height, INT width, INT esc,

This part looks good.
> diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
> (snip)
> +static void update_font_association_info(UINT current_ansi_codepage)
> +{
> +    static const WCHAR font_assoc_reg_keyW[] = {'S','y','s','t','e','m','\\',
> +        'C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t','\\',
> +        'C','o','n','t','r','o','l','\\','F','o','n','t','A','s','s','o','c','\0'};

It is nice introducing separate function for this work. However using
WCHAR is a bit overwork, because ANSI APIs are still used in
update_font_info() especially set_value_key(). Please follow the style
of surrounding code.
> @@ -3615,6 +3675,10 @@ static void update_font_info(void)
>      }
>      if (!done)
>          FIXME("there is no font defaults for codepages %u,%u\n", ansi_cp, oem_cp);
> +
> +    /* update locale dependent font association info in registry */
> +    if (strcmp(buf, cpbuf))
> +        update_font_association_info(ansi_cp);
>  }
>
> static BOOL init_freetype(void)

This condition should be rewritten. buf is not initialized when
RegQueryValueExA is failed in line 3542. And strcmp(buf, cpbuf) has
already done in line 3544.
> diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c
> index 943f584..9d57806 100644
> --- a/dlls/gdi32/tests/font.c
> +++ b/dlls/gdi32/tests/font.c

Why are there no tests against OEM_CHARSET and SYMBOL_CHARSET?

Regards,
Akihiro Sagawa

gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font associated charset

[Byeongsik Jeon]

Thank you.
Review again before submitting a patch to wine-patches, please.

On 02/26/13 00:08, Akihiro Sagawa wrote:
> On Sun, 24 Feb 2013 09:55:59 +0900, Byeongsik Jeon wrote:
> > @@ -3615,6 +3675,10 @@ static void update_font_info(void)
> >      }
> >      if (!done)
> >          FIXME("there is no font defaults for codepages %u,%u\n", ansi_cp, oem_cp);
> > +
> > +    /* update locale dependent font association info in registry */
> > +    if (strcmp(buf, cpbuf))
> > +        update_font_association_info(ansi_cp);
> >  }
> >
> > static BOOL init_freetype(void)
>
> This condition should be rewritten. buf is not initialized when
> RegQueryValueExA is failed in line 3542. And strcmp(buf, cpbuf) has
> already done in line 3544.

I wanted to update in registry when codepages changed, not logpixels. When
logpixels has been changed, MS-Windows does not update the font associated
charset registry. "memset(buf, 0, sizeof(buf))" has been added.
> > diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c
> > index 943f584..9d57806 100644
> > --- a/dlls/gdi32/tests/font.c
> > +++ b/dlls/gdi32/tests/font.c
>
> Why are there no tests against OEM_CHARSET and SYMBOL_CHARSET?

In the current Wine code, if lfCharset == OEM_CHARSET then GetTextCharset()
return a wrong value. This problem can be found in the test_oemcharset(). At
this point, I wanted to concentrate on the ANSI_CHARSET association problem.

gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font associated charset

[Akihiro Sagawa]

On Tue, 26 Feb 2013 10:42:14 +0900, Byeongsik Jeon wrote:
> > This condition should be rewritten. buf is not initialized when
> > RegQueryValueExA is failed in line 3542. And strcmp(buf, cpbuf) has
> > already done in line 3544.
>
> I wanted to update in registry when codepages changed, not logpixels. When
> logpixels has been changed, MS-Windows does not update the font associated
> charset registry. "memset(buf, 0, sizeof(buf))" has been added.

Good work. I got it.

Including other changes, it looks good for me.
Please send your patch to wine-patches.

Akihiro Sagawa