On Tue, Dec 4, 2018 at 12:08 PM Nikolay Sivov nsivov@codeweavers.com wrote:
I did some testing on Windows, and LB_INITSTORAGE is not supposed to work the way it's implemented right now, before or after your patch.
Summary of what I found:
- LB_INITSTORAGE should return current capacity, 'items_size' in our
case, not LB_OKAY;
- it's not additive, and calling it for 0 count does not increase
returned value, policy is items_size = max(items_size, wParam + 10);
- it never shrinks as far as I can tell from return value, items_size is
reset to 0 only on LB_RESETCONTENT, same for LB_DELETESTRING;
- array grows by 32 items, not 16;
- there is no alignment when growing on insert, so LB_RESETCONTENT ->
LB_INITSTORAGE(1) -> LB_ADDSTRING twice -> items_size is 33.
Currently we have no tests for this message.
Other than fixing the return value, I don't think that it matters much how Windows does it internally or the granularity. It's an implementation detail, more or less. I mean, this has been broken for ages, and I'm not aware of any bug or broken app because of it.
I think the policy you discovered sounds like a hack and probably there's something else (MSDN says it's also the number of items to add so it must be additive), I'll do more investigation on that.
For now, can we just fix the return value and perhaps change the granularity to 32 without all the other stuff that's inconsequential so we can get at least single-selection LBS_NODATA into Wine 4.0?
And if the whole granularity thing has to be changed (though I doubt it), it should just be fixed later, since it's totally separate issue anyway and more fragile. I mean this patch series won't break even current behavior (even if it's wrong), it will keep it the same so no regressions at all, and something always seems to pop up and may not make it to the code freeze at this rate,