Re: oleacc: implemented GetRoleText[A/W]

[Dmitry Timoshkov]

"Nikolay Sivov" <bunglehead(a)gmail.com> wrote:
> +BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason,
> +                    LPVOID lpvReserved)
> +{
> +    TRACE("%p, %d, %p\n", hinstDLL, fdwReason, lpvReserved);
> +
> +    switch (fdwReason)
> +    {
> +        case DLL_PROCESS_ATTACH:
> +            oleacc_handle = hinstDLL;
> +            break;
> +    }
> +    return TRUE;
> +}

Why don't you call DisableThreadLibraryCalls() if you don't use the thread
callouts?
> +UINT WINAPI GetRoleTextW(DWORD role, LPWSTR lpRole, UINT rolemax)
> +{
> +    HGLOBAL hmem;
> +    HRSRC hrsrc;
> +    unsigned int id;
> +    const WCHAR *p;
> +    INT ret;
> +
> +    TRACE("%u %p %u\n", role, lpRole, rolemax);
> +
> +    hrsrc = FindResourceW(oleacc_handle, MAKEINTRESOURCEW((LOWORD(role) >> 4) + 1),
> +                          (LPWSTR)RT_STRING);
> +    if (!hrsrc) return 0;
> +    hmem = LoadResource(oleacc_handle, hrsrc);
> +    if (!hmem) return 0;
> +
> +    p = LockResource(hmem);
> +    id = role & 0x000f;
> +    while (id--) p += *p + 1;
> +
> +    /* return role text length */
> +    if(!lpRole)
> +        return *p;
> +
> +    ret = LoadStringW(oleacc_handle, role, lpRole, rolemax);
> +    if(!(ret > 0))
> +        return 0;
> +
> +    return ret;
> +}

Why not just use LoadStringW directly?
> +UINT WINAPI GetRoleTextA(DWORD role, LPSTR lpRole, UINT rolemax)
> +{
> +    INT ret;
> +
> +    TRACE("%u %p %u\n", role, lpRole, rolemax);
> +
> +    /* get length (without trailing NULL) */
> +    ret = GetRoleTextW(role, NULL, rolemax);
> +
> +    if(!lpRole)
> +        return ret;

GetRoleTextW above should be called only for the !lpRole case. And better
to actually call GetRoleTextA.
> +    return LoadStringA(oleacc_handle, role, lpRole, min(rolemax, ret + 1));

Why not just use rolemax in the LoadStringA call?

Introducing the IDS_ROLE_SYSTEM_xxx identifiers is completely redundant,
use the ROLE_SYSTEM_xxx values directly instead.

-- 
Dmitry.