[1/2] richedit: Created initial tests for windowless richedit controls. (try 2)

[Dylan Smith]

On Sat, Nov 8, 2008 at 10:40 AM, Peter Oberndorfer <kumbayo84 at arcor.de>wrote:

> This page [1] seems to suggest memory returned from HeapAlloc
> is marked as not executable.
> Other tests (ntdll exception) are using VirtualAlloc()
> for dynamically created code.
>
> patch 1
> > +/* The following x86 code string converts between the thiscall and
> stdcall
> > + * calling convetions.  The thiscall calling convention places the This
> > + * pointer in ecx on the x86 platform, and the stdcall calling
> convention
> > + * pushes the This pointer on the stack as the first argument.
> > + *
> > + * The wrapper's code finishes by jumping to the real function.
> > + *
> > + * Byte codes are used so that a copy of it can be modified to use
> > + * for each method in ITextHost. */
>

Thanks, I forgot to address that issue because I was too busy trying to get
it to run correctly.  I used HeapAlloc initially because I wasn't familiar
with VirtualAlloc, so thanks for letting me know what to use.


> patch 2
> > -/* The following x86 code string converts between the thiscall and
> stdcall
> > +/* The following x86 code strings convert between the thiscall and
> stdcall
> >   * calling convetions.  The thiscall calling convention places the This
>
> Here you introduce comments that you seem to change in the second patch.
> Maybe use the right text in the first patch?
> Additionally the term code string sounds odd.
> Maybe replace that with "The following x86 assembler code..."
> or something similar?
>

All I changed in the comments between the patches was singular to plural
word changes.  Using the same comments for both patches would make the
comment confusing if only one of the patches were accepted.

I could change code string, but probably to "The following x86 byte
code...", since it is just the comments that are assembly.

>
>
> >  #define ITextServices_QueryInterface(p,a,b)
> (p)->lpVtbl->QueryInterface(p,a,b)
> >  #define ITextServices_AddRef(p) (p)->lpVtbl->AddRef(p)
> >  #define ITextServices_Release(p) (p)->lpVtbl->Release(p)
> > -/*** ITextServices methods ***/
> > -#define ITextServices_TxSendMessage(p,a,b,c,d)
> (p)->lpVtbl->TxSendMessage(p,a,b,c,d)
> > -#define ITextServices_TxDraw(p,a,b,c,d,e,f,g,h,i,j,k,l)
> (p)->lpVtbl->TxDraw(p,a,b,c,d,e,f,g,h,i,j,k,l)
>
> Why do you only move some of those #defines part of ITextServices
> and not the one part of IUnknown ?
> I do not know much about those #defines but this seems odd
>

The QueryInterface, AddRef, and Release methods were actually defined using
the stdcall calling convention, so it doesn't have the same problems as the
other methods which were defined using the C++ thiscall calling convention.

>
>
> Maybe i am missing something but why do you need those thiscall wrappers?
> Wouldn't it be possible to use
> the DEFINE_THISCALL_WRAPPER and THISCALL macros like in txtserv.c?
>

I can't use DEFINE_THISCALL_WRAPPER  because it uses __ASM_GLOBAL_FUNC,
__ASM_FUNC, and __ASM_NAME, which are defined in include/wine/port.h and
include/config.h.  These cannot be included in the tests since the tests
must be compiled and crosscompiled with the same configuration (and
include/config.h is generated by the configure script).  Previously I tried
to do it that way, but it wasn't commited I tried to define these macros in
the test without making it portable enough to other compilers.

>
> Greetings Peter
>

Thanks for all the helpful comments Peter!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.winehq.org/pipermail/wine-devel/attachments/20081108/e2faac36/attachment-0001.htm