RFC: More tests for riched20 ITextServices

[Dan Hipschman]

On Thu, Jul 03, 2008 at 05:32:32PM +1000, Austin Lund wrote:
> I have run these with the native dll in wine and it works fine, but
> would like someone to test them on windows platforms.

I couldn't get these to compile within a few minutes due to not being
familiar with what headers were needed, but I can comment on a few
things to get the ball rolling.

> +#include "config.h"
> +#include "wine/port.h"

These should not be included in tests.

> +    trace("(%p) QueryInterface requesting rrid %p -> ppvObject %p\n",
> +          This, riid, ppvObject);

Go easy on the trace calls in tests.

> +    *ppvObject = NULL;
> +
> +    if (IsEqualIID(riid, &IID_IUnknown))
> +        *ppvObject = (LPVOID)This;
> +    else if (IsEqualIID(riid, &IID_ITextHost))
> +        *ppvObject = (LPVOID)This;
> +    else
> +        return E_NOINTERFACE;
> +
> +    trace("ppvObject now %p\n",ppvObject);
> +    if (*ppvObject)
> +    {
> +        ITextHost_AddRef((ITextHost *)(*ppvObject));
> +        return S_OK;
> +    }
> +
> +    return E_NOINTERFACE;

This could be easily cleaned up.

> +    extern typeof(func) THISCALL(func);                                 \
> +                        __ASM_GLOBAL_FUNC(__thiscall_ ## func,          \
> +                                          "popl %eax\n\t"               \
> +                                          "pushl %ecx\n\t"              \
> +                                          "pushl %eax\n\t"              \
> +                                          "jmp " __ASM_NAME(#func) )

I would comment on the use of typeof, being a GCC extension, it should
be avoided.  I don't know why it's in the source already; unless we are
allowing them now maybe it just slipped by.

> +    dummyTextHost = CoTaskMemAlloc(sizeof(*dummyTextHost));
> +    if (dummyTextHost == NULL) {
> +	trace("Insufficient memory to create ITextHost interface\n");
> +	return FALSE;
> +    }

You can use skip for this sort of thing.

> +    init = CoTaskMemAlloc(sizeof(*init));
> +    if (init == NULL) {
> +	trace("Insufficient memory to create IUnknown interface\n");
> +	return FALSE;
> +    }
> +    result = CreateTextServices(NULL,(ITextHost*)dummyTextHost, &init);

Are you sure you need to allocate space for init here?  Usually in this
sort of situation you don't, and it makes little sense to allocate
sizeof(IUnknown) space since this is just an interface.

> +    ok(&(*txtserv) != NULL , "Could not get ITextServices interface\n");

This is a strange expression.  Wouldn't 'txtserv != NULL' work, and not
crash if txtserv is NULL?

Anyway, hope that helps, but I'm not too familiar with what you are
trying to do.  Maybe someone can give you more help on the core issues.