On Mon, Sep 10, 2018 at 11:40 AM, Huw Davies huw@codeweavers.com wrote:
Changing the type of len and making moving the declaration into an assignment don't belong in this patch. What you really want here is a check against LB_ERR, but that again is a different patch.
Oh I thought they were way too trivial changes to make in a separate patch, sorry. Though, indeed I'll add a check for LB_ERR in an extra patch then.
BTW (unrelated to this patch), I know you're reviewing the larger patches at the end of the patchset and want the function split into helper functions. I did a bit more this weekend and I want to emphasize again that the WM_CHAR will have to be rewritten at some point, so I think that it is the perfect opportunity to do it later rather than now. So IMO, if there aren't other issues, for now let's just try to get these committed as they are, so we can move to round 2 (and round 3 later, when the move to helper functions and the rewrite will happen).
I mean, I already have those patches written, since most of the WM_CHAR part (especially string enumeration) will have to be changed. But those patches will come after IACList::Expand (so probably on round 3, assuming this is round 1) when I will implement ResetEnumerator and the cached sorted string list (two of the TODO at the top of the file). I'd prefer if we do it then also because it would make around 30+ patches easier to rebase for me, sitting on way too many right now. :-)