Re: [paragui-users] Bugs in 1.0.3

[Alexander Pipelka]

Hi Mark!

Thanks for your help!

ad 1,2:
I fixed the PG_Widget userdata stuff. Definitely a bug :)

ad 3:
For the DEFINE thing i currently can't figure out what 
PARAGUI_DYNAMIC_EXPORTS should do. SDL defines the DECLSPEC macro to
handle symbol export. Your code would prevent dynamic export of symbols
if PARAGUI_DYNAMIC_EXPORTS is *not* defined. Hmm,...

ad 4:
I can see the point. I can see that your explanation should be right.
But after this I *don't* know why the current implementation has been
reported to work on Linux (GCC Versions: 2.91, 2.95, 3.0, 3.2), Windows
(VC++, DevC++ gcc 2.95, gcc 3.2, Code Warrior 7), BeOS, MacOS (Code
Warrior 7), MacOS X, FreeBSD.

I would be very glad if this could be fixed, but there's currently no
motivation for me because it just works for the platforms I'm using (I
know this attitude i technically not ok, but who cares).

Maybe an easy fix (but possibly breaking binary compatibility) would be
to subclass PG_MessageObject from PG_EventObject. This way all offsets
should be correct.

Alex

Am Sam, 2003-06-14 um 15.03 schrieb Mark Junker:
> Hi,
> 
> I currently received ParaGUI v1.0.3 and its a quite good work. Anyway, while  
> porting the sources to Borland C++ Builder 5, I've found several bugs in it:
> 
> 1. Wrong new instruction
> File: pgwidget.cpp
> Fn. : PG_Widget::SetUserData(void *userdata, int size)
> Line from: my_internaldata->userdata = new char(size);
>        to: my_internaldata->userdata = new char[size];
> 
> 2. Usage of free() instead of delete[]
> File: pgwidget.cpp
> Fn. : Destructor
> Line from: free(my_internaldata->userdata);
>        to: delete[] my_internaldata->userdata;
> 
> 3. Missing DEFINEs
> You don't use the DEFINE PARAGUI_DYNAMIC_EXPORTS.
> However, you should include the following lines into paraconfig_win32.h or
> similar:
> ------------------------------------------------------------------
> #ifdef DECLSPEC
> #undef DECLSPEC
> #endif
> 
> #ifdef PARAGUI_DYNAMIC_EXPORTS
> #define DECLSPEC __declspec(dllexport)
> #else
> #define DECLSPEC
> #endif
> ------------------------------------------------------------------
> 
> 4. Non-Working event mechanism
> When adding a function to the list of event handler using the following line
> (or something similar):
> ------------------------------------------------------------------
> m_pEditBox->SetEventObject(MSG_EDITEND, this, (MSG_CALLBACK_OBJ)
> &PG_SpinnerBox::handle_editend);
> ------------------------------------------------------------------
> The variable "this" will be converted to point to the beginning of the
> "PG_EventObject" portion of the current class. Depending on the
> compiler, it will be inserted at the beginning or appended to the end
> of the class.
> In other words: the conversion of "this" depends on the order of the
> parent classes.
> 
> Example:
> ------------------------------------------------------------------
> #include <stdio.h>
> class TTestBaseA {
>         int a;
> };
> class TTestBaseB {
>         int b;
> };
> void test_a(TTestBaseA *pTest) {
>         printf("Base of TTestBaseA* = %p\n", pTest);
> }
> void test_b(TTestBaseB *pTest) {
>         printf("Base of TTestBaseB* = %p\n", pTest);
> }
> class TTest : public TTestBaseA, public TTestBaseB {
>         int c;
>         void test(void) {
>                 test_a(this);
>                 test_b(this);
>         }
> };
> int main(int argc, char **argv) {
>         TTest t;
>         t.test();
>         return 0;
> }
> ------------------------------------------------------------------
> 
> As you can see, the pointers of TTestBaseA* and TTestBaseB* are different.
> The address of TTestBaseA* might be lower than TTestBaseB*, but you must not
> rely on it.
> 
> This behaviour is causing the crash, when the following line
> (pgmessageobject.cpp) will be executed:
> 
> rc = ((cbdata->calledobj)->*(cbdata->obj_cbfunc))(id, (PG_Widget*)this, data,
> cbdata->data);
> 
> The compiler thinks that obj_cbfunc points to a member function of
> PG_EventObject and therefore the address of "this" in the event handler is
> probably wrong (depending on the compiler/platform) because it doesn't point
> to the base of the - for example - PG_SpinnerBox object. It contains the
> address of the PG_EventObject inside this object.
> 
> I ran into this error when I used the Borland C++ Builder 5.
> 
> Please re-think the event mechanism because this is seriously a bug that has
> to be fixed.
> 
> What about using libsigc++ or a "generic" event handler like:
> virtual bool ProcessMessage(TEvent *pEvent);
> 
> where the ProcessMessage method tests for the event type and - if it is known
> - casts the pEvent pointer to TEventMouse or TEventKeyboard objects which are
> derived from TEvent and calls another virtual function that tries to handle
> this special event.
> 
> #define eET_Paint 1
> #define eET_MouseClick 2
> #define eET_MouseMove 3
> 
> struct TEvent {
>   int Type;
> };
> 
> struct TEventMouse {
>   bool Left;
>   bool Right;
>   bool Middle;
>   int  x, y;
> };
> 
> bool PG_MessageObject::ProcessMessage(TEvent *pEvent) {
>   /* do nothing */
>   return false;
> }
> 
> bool PG_Widget::ProcessMouseClick(TEventMouse *pEvent) {
>   return false;
> }
> 
> bool PG_Widget::ProcessMouseMove(TEventMouse *pEvent) {
>   return false;
> }
> 
> bool PG_Widget::ProcessMouseMessage(TEventMouse *pEvent) {
>   if (pEvent->Type==eET_MouseClick) {
>     return ProcessMouseClick(pEvent);
>   } else if (pEvent->Type==eET_MouseMove) {
>     return ProcessMouseMove(pEvent);
>   }
> }
> 
> bool PG_Widget::ProcessMessage(TEvent *pEvent) {
>   bool fResult = false;
>   switch (pEvent->Type) {
>   case eET_MouseClick:
>   case eET_MouseMove:
>     fResult = ProcessMouseMessage((TEventMouse*) pEvent);
>   }
>   if (!fResult) {
>     fResult = PG_MessageObject::ProcessMessage(pEvent);
>   }
>   return fResult;
> }
> 
> .
> .
> .
> .
> 
> and so on ...
> 
> I took a look at libsigc++ v1.2.5 - they use the same C++ language feature -
> and it only works because they
> 
> 1. store both object pointers (of SigC::Object and the user-defined class
> derived from SigC::Object)
> 2. pass both object pointers to a function (which stores them) after
> converting them to (void*) to get rid of implicit type casts
> 
> Maybe it's possible to make some small changes to get it up and running
> without seg faults ... but this mechanism doesn't seem to be very easy to
> maintain (just take a look at the configuration stuff of libsigc++).
> 
> Regards,
> Mark Junker
> 
> 
> _______________________________________________
> paragui-users mailing list
> address@hidden
> http://mail.nongnu.org/mailman/listinfo/paragui-users