On Wed, 13 May 2009, Pritpal Bedi wrote:

Hi,

> > METHOD New( oWnd, cProgID, nRow, nCol, nWidth, nHeight ) CLASS TActiveX
> >       ::oOleAuto = TOleauto():New( ActXPdisp( ::hActiveX ) )
> Apply this patch in olecore.c and let me know it solves this issue:
> Mindaugus, please check if this can be implemented like this or 
> some else construct is needed.
> HB_FUNC( OLECREATEOBJECT ) /* ( cOleName | cCLSID  [, cIID ] ) */
> {
>    wchar_t*    cCLSID;
>    GUID        ClassID, iid;
>    BOOL        fIID = FALSE;
>    IDispatch*  pDisp = NULL;
>    IDispatch** ppDisp;
>    const char* cOleName = hb_parc( 1 );
>    const char* cID = hb_parc( 2 );
>    HRESULT     lOleError;
> 
>    if( HB_IS_NUMBER( 1 ) )
>    {
>       IDispatch * pDisp = ( IDispatch * ) ( HB_PTRDIFF ) hb_parnint( 1 );
>       lOleError = pDisp->lpVtbl->AddRef( pDisp );

Now OLECREATEOBJECT( 1 ) cause GPF. Any code which makes casting from
numbers to pointers is potential source of GPF or memory corruption.
The second one is even worse because it may damage some other clean
subsystems, f.e. when user by mistake reuse old pointer stored as numeric
item and this memory was reused by some other code.
I hope that we clean our whole code and eliminate any peaces of such
conversion so we will not have even single ( HB_PTRDIFF ) hb_parnint( 1 )
or hb_retnint( ( HB_PTRDIFF ) ... ) instruction.

>       hb_setOleError( lOleError );
>       if( lOleError == S_OK )
>       {
>          hb_retnint( ( HB_PTRDIFF ) hb_param( 1, HB_IT_ANY ) );

Yet another example of bugs which can be introduce by such conversion.
You are returning pointer to some Harbour item converted to number.
I guess that it's a typo but if someone tries to reuse such pointer
as a number then he corrupts HVM internal structures. The bad thing is
that such code may not GPF so the exact buggy place will be very hard
to locate. If application uses a lot of such pointers as numbers and
some problems appear then it can be extremely hard to locate them
and fix. If the application is huge probably the easiest way will be
rewriting it from scratch.
I guess that you want to make sth like:
      hb_retnint( ( HB_PTRDIFF ) pDisp );
but it will not work with rest of OLE code which expect pointer item.
In summary I guess that you wanted to make sth like:

       if( lOleError == S_OK )
          lOleError = CoCreateInstance( HB_ID_REF( ClassID ), NULL, 
CLSCTX_SERVER, fIID ? HB_ID_REF( iid ) : HB_ID_REF( IID_IDispatch ), ( void** ) 
( void * ) &pDisp );
    }
+   else if( ISNUM( 1 ) )
+   {
+      pDisp = ( IDispatch * ) ( HB_PTRDIFF ) hb_parnint( 1 );
+#if HB_OLE_C_API
+      lOleError = ( HRESULT ) pDisp->lpVtbl->AddRef( pDisp );
+#else
+      lOleError = ( HRESULT ) pDisp->AddRef();
+#endif
+   }
    else
       lOleError = CO_E_CLASSSTRING;
   hb_setOleError( lOleError );

but there is a question if we want to still tolerate code with
C pointer <-> HVM number item conversions and keep workarounds
for it. I'm leaving the decision to Mindaugas. 

> I cannot test it because I cannot even compile with new SVN.

What's the exact problem?

best regards,
Przemek
_______________________________________________
Harbour mailing list
Harbour@harbour-project.org
http://lists.harbour-project.org/mailman/listinfo/harbour

Reply via email to