Re: StrCatChainW implementation

[Nikolay Sivov]

On 3/6/2014 15:17, Huw Campbell wrote:
> Hello
>
> Please see my implementation of this function, which  has not been 
> seen in wine previously. Explorer.exe calls it, so it's kind of important.
>
> I ran a lot of tests on the edge cases: including: same ends, 0 ends, 
> end beyond the length...
> Hi, Huw.

We need those tests in shlwapi/tests. Now regarding patch itself.
> +DWORD WINAPI StrCatChainW(PWSTR pszDst, DWORD cchDst, DWORD ichAt, PCWSTR pszSrc)
> Please use lower case naming that's more readable, like 'dest', 
> 'dest_len', 'at', 'src'.
> +{
> +  LPWSTR d = pszDst;
> +  LPCWSTR s = pszSrc;
> It doesn't look like you need this 'd' and 's', you could use passed 
> pointers.
> +  DWORD mark;
> +
> +  TRACE("(%s,%i,%i,%s)\n", debugstr_w(pszDst), cchDst, ichAt, debugstr_w(pszSrc));
> Is it possible that destination buffer is uninitialized? In this case 
> you can't trace in like this.
> +
> +  if (ichAt == -1)
> +    mark = strlenW(pszDst);
> +  else
> +    mark = ichAt;
> +  if (mark > cchDst || cchDst == 0)
> +    return mark;
> +  if (mark == cchDst)
> +    mark--;
> +  d = d + mark;
> +  if (pszSrc)
> +  {
> +    while ((mark < cchDst-1) && *s)
> +    {
> +      mark++;
> +      *d++ = *s++;
> +    }
> +  }
> +  *d = 0;
> What happens if 'd' is null? Could this loop be replaced with StrNCatW 
> or something similar?
> +  return mark;
> +}