[PyKDE] How to do operators with SIP

[Phil Thompson]

Donovan Rebbechi wrote:
> 
> On Fri, 16 Nov 2001, Phil Thompson wrote:
> 
> > PyNumberMethod __add__
> > %MemberCode
> >     QPoint *p0, *p1;
> >
> >     if ((p0 = (QPoint *)sipGetCppPtr((sipThisType *)a0,sipClass_QPoint))
> > == NULL)
> >         return NULL;
> 
> I find this idiom questionable for the following reason: the cast to
> sipThisType is unsafe, and the call to sipGetCppPtr (which doesn't do any
> error checking) results in a segfault, unless a0 is a "sip object" (as
> opposed to an ordinary PyObject*). So why check the return value of
> sipGetCppPtr if you're already assuming you know what a0 is ?

Yes, there could be better error checking. The programmer would have to
be deliberately messing with the internals for the object not to be of
the right type - the code should only ever be called from the SIP
generated Python code. The return value has to be checked because the
underlying C++ instance may have been deleted, even though the Python
object is still there.

> Part of the problem is the way sipGetCppPtr is implimented. A safer
> implementation of a similar thing is sipConvertToCpp, but this is
> unwieldy-- the third  argument does not seem necessary, since it always
> returns NULL if it fails.  A quick fix to avoid breaking things would be
> to allow a NULL pointer for the third argument of sipConvertToCpp ...
> 
> --- siplib.c    Sun Oct 28 04:53:19 2001
> +++ siplib.c.new        Sun Nov 18 01:44:23 2001
> @@ -2351,7 +2351,8 @@
> 
>         if ((sipThis = sipMapSelfToThis(sipSelf)) == NULL || (ptr =
> sipGetCppPtr(sipThis,baseclass)) == NULL)
>         {
> -               *iserrp = TRUE;
> +               if (iserrp != NULL)
> +                       *iserrp = TRUE;
>                 return NULL;
>         }
> 
> Then one could write code like this:
> 
> if ((p0 = (QPoint *)sipConvertToCpp(a0,sipClass_QPoint,NULL) == NULL )
>         return NULL;

I think a better solution is simply for sipConvertToCpp() to take a
PyObject* rather than a sipThisType* and add a check to test it really
is a sipThisType.

Phil