[issue2610] string representation of range

[Martin v. Löwis]

Martin v. Löwis <martin at v.loewis.de> added the comment:

> 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

----------
nosy: +loewis

__________________________________
Tracker <report at bugs.python.org>
<http://bugs.python.org/issue2610>
__________________________________