Re: [PATCH] comctl32/tests: Use HGLOBAL stream to Read/Write tests on imagelists

[Dmitry Timoshkov]

Nikolay Sivov  wrote:
> dlls/comctl32/tests/imagelist.c | 120 ++++++++++++++++++----------------------
>  1 file changed, 53 insertions(+), 67 deletions(-)

...
> -struct my_IStream
> +struct memstream
>  {
>      IStream IStream_iface;
> - char *iml_data; /* written imagelist data */
> - ULONG iml_data_size;
> - IStream *stream;
>
> };
>
> -static struct my_IStream *impl_from_IStream(IStream *iface)
> +static struct memstream *impl_from_IStream(IStream *iface)
>  {
> - return CONTAINING_RECORD(iface, struct my_IStream, IStream_iface);
> - return CONTAINING_RECORD(iface, struct memstream, IStream_iface);
>
> }

You could at least try to avoid unecessary structure renaming.
> static HRESULT STDMETHODCALLTYPE Test_Stream_QueryInterface(IStream *iface, REFIID riid,
> @@ -709,34 +708,15 @@ static ULONG STDMETHODCALLTYPE Test_Stream_Release(IStream *iface)
>  static HRESULT STDMETHODCALLTYPE Test_Stream_Read(IStream *iface, void *pv, ULONG cb,
>                                                    ULONG *pcbRead)
>  {
> - ok(0, "unexpected call\n");
> - return E_NOTIMPL;
>
> -}

This is not an improvement. The patch neither simplifies the code nor makes
it more readable. Besides the test is no longer able to detect that Read
method should not be called.

What is the idea behind this?