Jacek Caban wrote:
> Changelog:
> Unicodified fontdlg
>
>------------------------------------------------------------------------
>
>@@ -216,52 +213,60 @@
> */
> BOOL WINAPI ChooseFontA(LPCHOOSEFONTA lpChFont)
> {
>- LPCVOID template;
>- HRSRC hResInfo;
>- HINSTANCE hDlginst;
>- HGLOBAL hDlgTmpl;
>-
>- if ( (lpChFont->Flags&CF_ENABLETEMPLATEHANDLE)!=0 )
>- {
>- template=(LPCVOID)lpChFont->hInstance;
>- } else
>- {
>- if ( (lpChFont->Flags&CF_ENABLETEMPLATE)!=0 )
>- {
>- hDlginst=lpChFont->hInstance;
>- if( !(hResInfo = FindResourceA(hDlginst, lpChFont->lpTemplateName,
>- (LPSTR)RT_DIALOG)))
>- {
>- COMDLG32_SetCommDlgExtendedError(CDERR_FINDRESFAILURE);
>- return FALSE;
>- }
>- } else
>- {
>- hDlginst=COMDLG32_hInstance;
>- if (!(hResInfo = FindResourceA(hDlginst, "CHOOSE_FONT", (LPSTR)RT_DIALOG)))
>- {
>- COMDLG32_SetCommDlgExtendedError(CDERR_FINDRESFAILURE);
>- return FALSE;
>- }
>- }
>- if (!(hDlgTmpl = LoadResource(hDlginst, hResInfo )) ||
>- !(template = LockResource( hDlgTmpl )))
>- {
>- COMDLG32_SetCommDlgExtendedError(CDERR_LOADRESFAILURE);
>- return FALSE;
>- }
>+ CHOOSEFONTW chFontw;
>+ LOGFONTW logFontw;
>+ LPLOGFONTA lpLogFonta;
>+ int len;
>+ BOOL ret;
>+ LPSTR lpszStyle;
>+ LPCSTR lpTemplateName;
>+
>+ memcpy(&chFontw, lpChFont, sizeof(CHOOSEFONTW));
>+ memcpy(&logFontw, lpChFont->lpLogFont, sizeof(LOGFONTA));
>+ chFontw.lpLogFont = &logFontw;
>+
>+ MultiByteToWideChar(CP_ACP, 0, lpChFont->lpLogFont->lfFaceName,
>+ LF_FACESIZE, chFontw.lpLogFont->lfFaceName, LF_FACESIZE);
>+
>+ if(lpChFont->lpszStyle) {
>+ len = MultiByteToWideChar(CP_ACP, 0, lpChFont->lpszStyle, -1, NULL, 0);
>+ chFontw.lpszStyle = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, len*sizeof(WCHAR));
>
>
Initialising the whole string to zero just before it is about to be
overwritten seems needlessly wasteful on CPU cycles and memory
bandwidth. You seem to do this in a few places. Yes, MultiByteToWideChar
can, in some cases, leave the string without a null terminator, but that
only happens when there is insufficient buffer. In this case, the only
way that can happen is if another thread modifies the buffer between the
previous MultiByteToWideChar and the next one here. If you wanted to be
really careful, you could always add "chFontw.lpszStyle[len-1] = '\0';"
after to call to MultiByteToWideChar.
>+ MultiByteToWideChar(CP_ACP, 0, lpChFont->lpszStyle, -1, chFontw.lpszStyle, len);
>+ HeapFree(GetProcessHeap(), 0, lpChFont->lpszStyle);
>
>
You seem to be freeing a buffer that the user passed in. How do you know
it was allocated with HeapAlloc?
>+ }
>+
>+ if(lpChFont->lpTemplateName) {
>+ len = MultiByteToWideChar(CP_ACP, 0, lpChFont->lpTemplateName, -1, NULL, 0);
>+ chFontw.lpTemplateName = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, len*sizeof(WCHAR));
>+ MultiByteToWideChar(CP_ACP, 0, lpChFont->lpTemplateName,
>+ -1, (LPWSTR)chFontw.lpTemplateName, len);
>+ }
>+
>+ ret = ChooseFontW(&chFontw);
>+
>+ lpLogFonta = lpChFont->lpLogFont;
>+ lpszStyle = lpChFont->lpszStyle;
>+ lpTemplateName = lpChFont->lpTemplateName;
>+ memcpy(lpChFont, &chFontw, sizeof(CHOOSEFONTA));
>+ lpChFont->lpLogFont = lpLogFonta;
>+ lpChFont->lpszStyle = lpszStyle;
>+ lpChFont->lpTemplateName = lpTemplateName;
>+ memcpy(lpChFont->lpLogFont, &logFontw, sizeof(LOGFONTA));
>+ WideCharToMultiByte(CP_ACP, 0, logFontw.lfFaceName,
>+ LF_FACESIZE, lpChFont->lpLogFont->lfFaceName, LF_FACESIZE, 0, 0);
>+
>+ if(chFontw.lpszStyle) {
>+ len = WideCharToMultiByte(CP_ACP, 0, chFontw.lpszStyle, -1, NULL, -1, 0, 0);;
>+ WideCharToMultiByte(CP_ACP, 0, chFontw.lpszStyle, -1, lpChFont->lpszStyle, len, 0, 0);
>+ HeapFree(GetProcessHeap(), 0, chFontw.lpszStyle);
> }
>- if (TRACE_ON(commdlg))
>- _dump_cf_flags(lpChFont->Flags);
>
>- if (lpChFont->Flags & (CF_SELECTSCRIPT | CF_NOVERTFONTS ))
>- FIXME(": unimplemented flag (ignored)\n");
>+ if(chFontw.lpTemplateName)
>+ HeapFree(GetProcessHeap(), 0, (LPBYTE)chFontw.lpTemplateName);
>
>
You shouldn't need a cast here, unless lpTemplateName is a const.
>
>- return DialogBoxIndirectParamA(COMDLG32_hInstance, template,
>- lpChFont->hwndOwner, FormatCharDlgProcA, (LPARAM)lpChFont );
>+ return ret;
> }
>
>-
> #define TEXT_EXTRAS 4
> #define TEXT_COLORS 16
>
>
>
...
>@@ -1072,34 +1083,34 @@
> }
>
> /***********************************************************************
>- * FormatCharDlgProcA [internal]
>+ * FormatCharDlgProc [internal]
> */
>-INT_PTR CALLBACK FormatCharDlgProcA(HWND hDlg, UINT uMsg, WPARAM wParam,
>+INT_PTR CALLBACK FormatCharDlgProc(HWND hDlg, UINT uMsg, WPARAM wParam,
> LPARAM lParam)
> {
>- LPCHOOSEFONTA lpcf;
>+ LPCHOOSEFONTW lpcf;
> INT_PTR res = FALSE;
>
> if (uMsg!=WM_INITDIALOG)
> {
>- lpcf=(LPCHOOSEFONTA)GetPropA(hDlg, WINE_FONTDATA);
>- if (!lpcf && uMsg != WM_MEASUREITEM)
>+ lpcf=(LPCHOOSEFONTW)GetPropW(hDlg, strWineFontData);
>+ if (!lpcf)
> return FALSE;
> if (CFn_HookCallChk32(lpcf))
>- res=CallWindowProcA((WNDPROC)lpcf->lpfnHook, hDlg, uMsg, wParam, lParam);
>+ res=CallWindowProcW((WNDPROC)lpcf->lpfnHook, hDlg, uMsg, wParam, lParam);
>
>
This isn't something wrong with your patch, but is in the original code
too. We shouldn't need to cast here, because if lpfnHook isn't using the
right calling conventions or doesn't have the same number of parameters
then we shouldn't be using CallWindowProc to start with.
> if (res)
> return res;
> }
> else
> {
>- lpcf=(LPCHOOSEFONTA)lParam;
>+ lpcf=(LPCHOOSEFONTW)lParam;
> if (!CFn_WMInitDialog(hDlg, wParam, lParam, lpcf))
> {
> TRACE("CFn_WMInitDialog returned FALSE\n");
> return FALSE;
> }
> if (CFn_HookCallChk32(lpcf))
>- return CallWindowProcA((WNDPROC)lpcf->lpfnHook,hDlg,WM_INITDIALOG,wParam,lParam);
>+ return CallWindowProcW((WNDPROC)lpcf->lpfnHook,hDlg,WM_INITDIALOG,wParam,lParam);
> }
> switch (uMsg)
> {
>
>
Rob