[PATCH] atl: Fix the ATL_WNDCLASSINFOW::m_szAutoName member definition and construction.
Zebediah Figura
z.figura12 at gmail.com
Wed Jan 15 16:31:57 CST 2020
Hello Hermès, a few comments.
On 1/15/20 3:59 PM, Hermès BÉLUSCA-MAÏTO wrote:
> From 0e829de8bd04d42fe2dca688ebd1799ee5ffe048 Mon Sep 17 00:00:00 2001
> From: Hermes Belusca-Maito <hermes.belusca at sfr.fr>
> Date: Wed, 15 Jan 2020 02:09:33 +0100
> Subject: [PATCH] atl: Fix the ATL_WNDCLASSINFOW::m_szAutoName member
> definition and construction.
>
> - Document the m_szAutoName size.
>
> - Make the format actually MS-compatible: "ATL" followed by colon,
> followed by hexadecimal digits of pointer (4*2 digits on 32-bit platforms,
> 8*2 digits on 64-bit platforms). This correctly fix x64 compilation
> and still make the autogenerated name "unique".
>
> - Add a test that checks the name prefix.
>
> Signed-off-by: Hermes Belusca-Maito <hermes.belusca at sfr.fr>
> ---
> dlls/atl/atl30.c | 10 +++----
> dlls/atl/tests/module.c | 59 +++++++++++++++++++++++++++++++++++++++++
> include/atlwin.h | 4 +--
> 3 files changed, 66 insertions(+), 7 deletions(-)
>
> diff --git a/dlls/atl/atl30.c b/dlls/atl/atl30.c
> index 29b29d4cbe..ee1b09cdac 100644
> --- a/dlls/atl/atl30.c
> +++ b/dlls/atl/atl30.c
> @@ -313,7 +313,7 @@ ATOM WINAPI AtlModuleRegisterWndClassInfoA(_ATL_MODULEA *pm, _ATL_WNDCLASSINFOA
>
> if (!wci->m_wc.lpszClassName)
> {
> - sprintf(wci->m_szAutoName, "ATL%08x", PtrToUint(wci));
> + sprintf(wci->m_szAutoName, "ATL:%p", wci);
> TRACE("auto-generated class name %s\n", wci->m_szAutoName);
> wci->m_wc.lpszClassName = wci->m_szAutoName;
> }
> @@ -350,8 +350,8 @@ ATOM WINAPI AtlModuleRegisterWndClassInfoA(_ATL_MODULEA *pm, _ATL_WNDCLASSINFOA
> * NOTES
> * Can be called multiple times without error, unlike RegisterClassEx().
> *
> - * If the class name is NULL, then a class with a name of "ATLxxxxxxxx" is
> - * registered, where the 'x's represent a unique value.
> + * If the class name is NULL, then a class with a name of "ATL:xxxxxxxx" is
> + * registered, where 'xxxxxxxx' represents a unique hexadecimal value.
> *
> */
> ATOM WINAPI AtlModuleRegisterWndClassInfoW(_ATL_MODULEW *pm, _ATL_WNDCLASSINFOW *wci, WNDPROC *pProc)
> @@ -372,8 +372,8 @@ ATOM WINAPI AtlModuleRegisterWndClassInfoW(_ATL_MODULEW *pm, _ATL_WNDCLASSINFOW
>
> if (!wci->m_wc.lpszClassName)
> {
> - static const WCHAR szFormat[] = {'A','T','L','%','0','8','x',0};
> - swprintf(wci->m_szAutoName, ARRAY_SIZE(wci->m_szAutoName), szFormat, PtrToUint(wci));
> + static const WCHAR szFormat[] = {'A','T','L',':','%','p',0};
> + swprintf(wci->m_szAutoName, ARRAY_SIZE(wci->m_szAutoName), szFormat, wci);
While you're at it you might as well use a wide string constant here,
since this module is built with msvcrt.
> TRACE("auto-generated class name %s\n", debugstr_w(wci->m_szAutoName));
> wci->m_wc.lpszClassName = wci->m_szAutoName;
> }
> diff --git a/dlls/atl/tests/module.c b/dlls/atl/tests/module.c
> index 192b23ef1e..021846ccaf 100644
> --- a/dlls/atl/tests/module.c
> +++ b/dlls/atl/tests/module.c
> @@ -25,6 +25,7 @@
> #define COBJMACROS
>
> #include <atlbase.h>
> +#include <atlwin.h>
>
> #include <wine/test.h>
>
> @@ -113,6 +114,63 @@ static void test_winmodule(void)
> ok(winmod.m_pCreateWndList == create_data+1, "winmod.m_pCreateWndList != create_data\n");
> }
>
> +static void test_winclassinfo(void)
> +{
> + _ATL_MODULEA winmod;
> + HRESULT hres;
> + size_t len, expectedLen;
> + ATOM atom;
> + WNDPROC wndProc;
> + _ATL_WNDCLASSINFOA wci =
> + {
> + /* WNDCLASSEXA m_wc; */
> + {
> + sizeof(WNDCLASSEXA),
> + CS_VREDRAW | CS_HREDRAW,
> + DefWindowProcA,
> + 0,
> + 0,
> + NULL,
> + NULL,
> + LoadCursorA(NULL, IDC_ARROW),
> + (HBRUSH)(COLOR_BTNFACE + 1),
> + NULL,
> + NULL, /* LPCSTR lpszClassName; <-- We force ATL class name generation */
> + NULL
> + },
> + NULL, /* LPCSTR m_lpszOrigName; */
> + NULL, /* WNDPROC pWndProc; */
> + IDC_ARROW, /* LPCSTR m_lpszCursorID; */
> + TRUE, /* BOOL m_bSystemCursor; */
> + 0, /* ATOM m_atom; */
> + "" /* CHAR m_szAutoName[...]; */
> + };
I'm a strong proponent of designated initializers for cases like this.
> +
> + winmod.cbSize = sizeof(winmod);
> + winmod.m_pCreateWndList = (void*)0xdeadbeef;
> + hres = AtlModuleInit(&winmod, NULL, NULL);
> + ok(hres == S_OK, "AtlModuleInit failed: %08x\n", hres);
> + ok(!winmod.m_pCreateWndList, "winmod.m_pCreateWndList = %p\n", winmod.m_pCreateWndList);
> +
> + atom = AtlModuleRegisterWndClassInfoA(&winmod, &wci, &wndProc);
> + ok(atom != NULL, "AtlModuleRegisterWndClassInfoA failed: %08x\n", atom);
> + ok(atom == wci.m_atom, "(atom = %08x) is != than (wci.m_atom = %08x)\n", atom, wci.m_atom);
> +
> + /* Check for the prefix */
> + ok(strncmp(wci.m_szAutoName, "ATL:", 4) == 0, "wci.m_szAutoName = '%s', expected starting with 'ATL:'\n", wci.m_szAutoName);
> +
> + /* Be sure m_szAutoName is NULL-terminated as it should by checking for NULL-terminator at its end - the string in it should always have the same length */
> + ok(wci.m_szAutoName[ARRAY_SIZE(wci.m_szAutoName) - 1] == 0, "wci.m_szAutoName is not NULL-terminated!\n");
> +
> + /* Manually NULL-terminate it in case something bad happened */
> + wci.m_szAutoName[ARRAY_SIZE(wci.m_szAutoName) - 1] = 0;
I don't think there's much point working around failures we don't expect
to happen. If the string is not null-terminated the above test will fail
and we'll know.
> +
> + /* Verify its length */
> + len = strlen(wci.m_szAutoName);
> + expectedLen = sizeof("ATL:") + sizeof(void *) * 2 - 1;
> + ok(len == expectedLen, "wci.m_szAutoName has length %d, expected length %d\n", len, expectedLen);
Do you really need all of these comments? The code seems clear enough.
> +}
> +
> static DWORD cb_val;
>
> static void WINAPI term_callback(DWORD dw)
> @@ -156,5 +214,6 @@ START_TEST(module)
> {
> test_StructSize();
> test_winmodule();
> + test_winclassinfo();
> test_term();
> }
> diff --git a/include/atlwin.h b/include/atlwin.h
> index 386177ba61..2ad812b201 100644
> --- a/include/atlwin.h
> +++ b/include/atlwin.h
> @@ -29,7 +29,7 @@ typedef struct _ATL_WNDCLASSINFOA_TAG
> LPCSTR m_lpszCursorID;
> BOOL m_bSystemCursor;
> ATOM m_atom;
> - CHAR m_szAutoName[14];
> + CHAR m_szAutoName[sizeof("ATL:") + sizeof(void *) * 2]; // == 4 characters + NULL + number of hexadecimal digits describing a pointer.
> } _ATL_WNDCLASSINFOA;
>
> typedef struct _ATL_WNDCLASSINFOW_TAG
> @@ -40,7 +40,7 @@ typedef struct _ATL_WNDCLASSINFOW_TAG
> LPCWSTR m_lpszCursorID;
> BOOL m_bSystemCursor;
> ATOM m_atom;
> - WCHAR m_szAutoName[14];
> + WCHAR m_szAutoName[sizeof("ATL:") + sizeof(void *) * 2]; // == 4 characters + NULL + number of hexadecimal digits describing a pointer.
> } _ATL_WNDCLASSINFOW;
>
> ATOM WINAPI AtlModuleRegisterWndClassInfoA(_ATL_MODULEA *pm, _ATL_WNDCLASSINFOA *wci, WNDPROC *pProc);
In my copy of atlwin.h (from the Windows 7 DDK) this is sizeof("ATL:") +
sizeof(void *) * 2 + 1, i.e. one more than you have here.
> --
> 2.22.0.windows.1
>
More information about the wine-devel
mailing list