Re: [PATCH] comctl32: Avoid an unneeded lstrlenW() call.

[Francois Gouget]

On Mon, 22 Oct 2018, Dmitry Timoshkov wrote:
> Zhiyi Zhang  wrote:
> > > @@ -2435,7 +2435,7 @@ TOOLBAR_CustomizeDialogProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
> > >                      btnInfo->btn = nmtb.tbButton;
> > >                      if (!(nmtb.tbButton.fsStyle & BTNS_SEP))
> > >                      {
> > > -
> > >                    if (lstrlenW(nmtb.pszText))
> > > -
> > >                    if (*nmtb.pszText)]
> >
> > There is a behavior change with this modification. lstrlenW won't crash even if applications somehow
> > replaced pszText with null. So we might want to check that pszText is not null before deferencing it.
>
> Have a look at include/winbase.h: lstr* APIs in Wine code are silently
> replaced by inline versions without an exception handler.

Right. In the patch I forgot to mention that I checked Wine's 
lstrlen{A,W}() implementation in winbase.h but I forgot to check what 
the MSDN says (or the kernel32/string.c implementation). Given the 
inconsistency that's all the more reason to avoid using lstrlen{A,W}() 
in such a case (or at all actually).

In this case nmtb.pszText is set to Buffer which is a WCHAR Buffer[256]. 
So it cannot be NULL and MSDN's lstrlenW() exception handler would not 
be needed anyway.