On Wednesday 23 January 2008 02:11, Kirill wrote: > 2. Could someone please review the following patch (size is the size > of a string buffer, passed to IContextMenu::GetCommandString): > diff --git a/menu.c b/menu.c > index 544be7e..53a1190 100644 > --- a/menu.c > +++ b/menu.c > @@ -185,9 +185,9 @@ static STDMETHODIMP get_command_string(void *p, UINT > id, LPWSTR tw = malloc((strlen(text)+1)*sizeof(wchar_t)); mbstowcs(tw, > text, strlen(text)); > if (flags & GCS_UNICODE) > - lstrcpynW((LPWSTR)name, tw, size); > + lstrcpynW((LPWSTR)name, tw, strlen(text)); > else > - lstrcpynA(name, text, size); > + lstrcpynA(name, text, strlen(text)); > free(tw); > return S_OK; > }
This is not correct for two reasons: - You cannot derive the number of Unicode characters from the result of strlen(). This does not work for multi-byte character sequences (e.g. Japanese Shift-JIS encoded strings). Use should use lstrlenW(tw) instead if there were not error #2: - lstrcpyn() expects the maximum number of characters to copy as the 3rd parameter, including the NUL character, and that must be the space that is available. Hence the original, size, is correct. BTW, lstrcpyn is much better than strncpy because it properly NUL terminates the destination (and does not fill the buffer with NULs if the source is short). -- Hannes
