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;
+}