Re: oleacc: implemented GetRoleText[A/W]

[Nikolay Sivov]

Dmitry Timoshkov wrote:
> "Nikolay Sivov"  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?

Will add it.
> > +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?

Because I don't know how to get string length with LoadStringW. It 
returns with zero when
called with NULL buffer. I could pass a pointer to single char and 
(buflen = = 0) maybe.
Btw, is it normal that LoadString[A/W] differs much handling this 
NULL-buffer cases?
I've check just now that there's no separate test for NULL buffer for 
LoadStringA.
> > +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.

I can get rid of that after testing LoadStringA.
> > - return LoadStringA(oleacc_handle, role, lpRole, min(rolemax, ret
> > - 1));
>
> Why not just use rolemax in the LoadStringA call?

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

You mean to use ROLE_SYSTEM_* directly in all .rc files?