[Python-3000] [Python-3000-checkins] r54588

[Guido van Rossum]

Thanks Amaury! I've submitted your fix:
Committed revision 54609.

(Brett's fix would require an additional DECREF when NotImplemented is
returned, so Amaury's version is better. I bet a compiler can
rearrange the code so that the INCREF code is shared. :-)

--Guido

On 3/29/07, Brett Cannon <brett at python.org> wrote:
> On 3/29/07, Amaury Forgeot d'Arc <amauryfa at gmail.com> wrote:
> > Hello,
> >
> > Sorry if I am wrong, but it seems to me that the change in r54588 has a problem:
> > http://mail.python.org/pipermail/python-3000-checkins/2007-March/000433.html
> > - in the normal case, the return value is INCREF'ed twice
> > - in the error case, Py_INCREF(NULL) is called...
> >
> > One easy way to correct this is to move the last INCREF:
> > (sorry for the approximative patch format)
> >
> > python/branches/p3yk/Objects/typeobject.c:
> > =================================================
> >  static PyObject *
> >  object_richcompare(PyObject *self, PyObject *other, int op)
> >  {
> >         PyObject *res;
> >
> >         switch (op) {
> >
> >         case Py_EQ:
> >                 res = (self == other) ? Py_True : Py_False;
> > +               Py_INCREF(res);
> >                 break;
> >
> >         case Py_NE:
> >                 /* By default, != returns the opposite of ==,
> >                    unless the latter returns NotImplemented. */
> >                 res = PyObject_RichCompare(self, other, Py_EQ);
> >                 if (res != NULL && res != Py_NotImplemented) {
> >                         int ok = PyObject_IsTrue(res);
> >                         Py_DECREF(res);
> >                         if (ok < 0)
> >                                 res = NULL;
> >                         else {
> >                                 if (ok)
> >                                         res = Py_False;
> >                                 else
> >                                         res = Py_True;
> >                                 Py_INCREF(res);
> >                         }
> >                 }
> >                 break;
> >
> >         default:
> >                 res = Py_NotImplemented;
> > +               Py_INCREF(res);
> >                 break;
> >         }
> >
> > -       Py_INCREF(res);
> >         return res;
> >  }
> >
>
>
> It looks right, although you could also remove the INCREF in Py_NE and
> change the INCREF at the bottom to XINCREF or just return on the
> error.
>
> -Brett
> _______________________________________________
> Python-3000 mailing list
> Python-3000 at python.org
> http://mail.python.org/mailman/listinfo/python-3000
> Unsubscribe: http://mail.python.org/mailman/options/python-3000/guido%40python.org
>


-- 
--Guido van Rossum (home page: http://www.python.org/~guido/)