This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author loewis
Recipients benjamin.peterson, bmiller, loewis
Date 2008-04-10.21:13:31
SpamBayes Score 0.0073698466
Marked as misclassified No
Message-id <47FE82F9.2010304@v.loewis.de>
In-reply-to <1207858708.98.0.537185652213.issue2610@psf.upfronthosting.co.za>
Content
> I use Python in my CS1 and CS2 curriculum and I have a question.

At this point, I would have almost stopped reading - the tracker
is not a place for questions. I read on and then noticed that you
have a patch also, not just a question :-)

> This is my first attempt at patching any part of the C code for Python.  
> Please let me know what should be changed and If I've missed something.

The formatting of the C code is wrong in on place (there must not be a
space after the opening or before the closing parenthesis); I would also
drop the space after the < and before the > in the result.

Please try to avoid replicating code as much as possible. Rather than
this complex muliply/add code, use range_item(r, 1), range_item(r, 2),
range_item(r, -2), range_item(r, -1). Use range_length to find out how
many things are in the range, and then again range_item for the 0..7th
element.

Don't use PyUnicode_FromFormat for a single %R; use PyObject_Repr
instead.

The code leaks many object references. Almost any function you call
produces a new reference, and you have to decref it after you don't
need the value anymore. E.g. write

  repr = PyObject_Repr(nth);
  appended = PyUnicode_Concat(result, repr);
  Py_DECREF(repr); // not needed anymore, so release it
  Py_DECREF(result); // about to assign to result, so release old value
  result = appended;

Consider exceptions. About any function returning PyObject* can fail
with an exception, if for not other reason than being out-of-memory.
Don't forget to release partial results in case an exception, making
your code like

  repr = PyObject_Repr(nth);
  if (!repr)
    goto fail;
  appended = PyUnicode_Concat(result, repr);
  if (!appended)
    goto fail;
  Py_DECREF(repr);
  Py_DECREF(result);
  result = appended;
...
fail:
  Py_XDECREF(repr);
  Py_XDECREF(appended);
  ...
  return NULL;

As we don't know in the failure case easily whether appended was already
assigned to, we use XDRECREF; don't forget to initialize appended with
NULL.

As both the success case and the failure case end with a long list
of DECREFs, it is common to phrase that as

  PyObject *result = NULL;
  ...
  result = what_shall_i_really_return;
  what_shall_i_really_return = NULL;
fail:
  lots_of_xdecrefs;
  return result;

In your case, it might be useful to create an array of up to 7
PyObject*, zero-initialize that, and put the partial results in
there; then XDECREF that in a loop at the end.

HTH,
Martin
History
Date User Action Args
2008-04-10 21:13:33loewissetspambayes_score: 0.00736985 -> 0.0073698466
recipients: + loewis, benjamin.peterson, bmiller
2008-04-10 21:13:31loewislinkissue2610 messages
2008-04-10 21:13:31loewiscreate