[paragui-users] Bugs in 1.0.3

[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