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