[PATCH 1/2] comctl32/tests: test toolbar separator with uninitialized iString

[Giuseppe Bilotta]

2009/4/23 Mikołaj Zalewski <mikolaj at zalewski.pl>:
>  Hi,
>  I think it would be interesting to test what TB_GETBUTTONINFO returns as
> iString for such a separator. That way we will know if the value is simply
> ignored, or it's something more complicated.

You're definitely on to something here. Indeed, trying to get the TEXT
button info causes a crash in Windows XP and Vista. Of course, during
normal usage nobody is going to have a look at the iString of a
separator.

>> +static void test_deadbeef(void)
>>
>
>  I think you should find a better name. Maybe test_addbuttons would be
> better - some more tests for this message could be added later?

As per your suggestion, I named it test_invalidstring

> Tests with #if 0 are discouraged - usually after some time and changes in
> the surrounding, the code doesn't compile anymore. Probably leaving only the
> comment would be appropriate in this case.

On #wine-devel it was suggested that #if 0 is the appropriate choice
for tests that (can) crash on Windows. I think that having the code
around, ready for testing, would be appropriate, at the very least
until we decide what the Wine behaviour in these cases should be.

> It's interesting that the
> behaviour changed with the version of Windows. You were testing with a
> program without a manifest? (i.e. using comctl32 v5.82. But it's hard to
> make a mistake, as if you used a manifest, you would have a lot of failures
> in check_sizes)

I'm testing using comctl32_crosstest as built by make crosstest, using
mingw32 shipped by debian for x86-64. I assume that one doesn't have a
manifest.

It would seem that Windows XP (1) doesn't bother checking the pointers
at all and (2) doesn't access the pointer itself until the button
label is needed. Windows 2008 and Windows Vista instead access the
pointer at button insertion time, but not when the button is a
separator. Additionally, Windows 2008 does not check the pointer.
Instead, Windows Vista checks the pointer and makes the insertion fail
when it's invalid.

I think that Vista's (apparent) approach is the most sensible: abort
insertion without crashing on invalid pointer, unless we're handling a
separator. Accept a separator with invalid address, keeping the
address as-is. This might lead to a crash subsequently, if the text is
accessed, but that happens in Windows too so it's fine.

-- 
Giuseppe "Oblomov" Bilotta