C-API: A beginner's problem

[Fabian Steiner]

I recently started learning C since I want to be able to write Python
extension modules. In fact, there is no need for it, but I simply want
to try something new ...

I tried to implement the bubblesort algorithm in C and to use it in
python; bubblesort.c compiles fine, but whenever I want to import the
modul and call the function I get a segmentation fault. This is what the
code looks like:

static PyObject *py_bubblesort(PyObject *self, PyObject *args) {
PyObject *seq = NULL, *item, *newseq = NULL;
int seqlen, i;

if(!PyArg_ParseTuple(args, "O", &seq)) {
return NULL;
}
seq = PySequence_Fast(seq, "argument must be iterable");
if(!seq) {
return NULL;
}
seqlen = PySequence_Fast_GET_SIZE(seq);
int list[seqlen];
for (i = 0; i <= seqlen; i++) {
item = PySequence_Fast_GET_ITEM(seq, i);
list = item;
}
bubblesort(list, seqlen);
newseq = PyList_New(seqlen);
if(!newseq) {
return NULL;
}
for(i = 0; i < seqlen; i++) {
PyList_SetItem(newseq, i, list);
}

return newseq;

bubblesort(int list[], int seqlen) is doing the actual job and it is
working.

What did I do wrong? As I am quite new to C, I probably made many
mistakes, so please feel free to correct me.

Cheers,
Fabian

 

[Heikki Salo]

What did I do wrong? As I am quite new to C, I probably made many
mistakes, so please feel free to correct me.

The following line:

> for (i = 0; i <= seqlen; i++) {

Should be "for (i = 0; i < seqlen; i++) {". Otherwise the last
assignment will be out of bounds and probably corrupts heap.

 

[Heikki Salo]

The following line:


Should be "for (i = 0; i < seqlen; i++) {". Otherwise the last
assignment will be out of bounds and probably corrupts heap.

And closer look tells that the code should not even compile. Is the code
cut & pasted directly? Line "list = item;" tries to assign a pointer
to an int-array, which should not compile. There are other similar oddities.

 

[Duncan Booth]

And closer look tells that the code should not even compile. Is the
code cut & pasted directly? Line "list = item;" tries to assign a
pointer to an int-array, which should not compile. There are other
similar oddities.

.... such as the declaration of list at a point in the code which is not
permitted in C, and using a non constant value for the length (which is
also not allowed).

 

[Nick Smallbone]

And closer look tells that the code should not even compile. Is the
code cut & pasted directly? Line "list = item;" tries to assign a
pointer to an int-array, which should not compile. There are other
similar oddities.

... such as the declaration of list at a point in the code which is not
permitted in C, and using a non constant value for the length (which is
also not allowed).

They are allowed in C99, according to GCC's manual.

 

[Duncan Booth]

And closer look tells that the code should not even compile. Is the
code cut & pasted directly? Line "list = item;" tries to assign a
pointer to an int-array, which should not compile. There are other
similar oddities.

... such as the declaration of list at a point in the code which is not
permitted in C, and using a non constant value for the length (which is
also not allowed).

They are allowed in C99, according to GCC's manual.

Which just shows how long it is since I wrote any C.

 

[Fabian Steiner]

The following line:


Should be "for (i = 0; i < seqlen; i++) {". Otherwise the last
assignment will be out of bounds and probably corrupts heap.

And closer look tells that the code should not even compile. Is the code
cut & pasted directly? Line "list = item;" tries to assign a pointer
to an int-array, which should not compile. There are other similar
oddities.

Okay, thank you (and the others) for these hints. As you see, I am quite
new to C and I just wrote these lines by using parts I have found in the
newsgroup. Unfortunately, the Python C-API documentation / tutorial
won't help me since it is quite difficult to understand because of the
lack of basics.

What do I have to change in order to make the code work?

Thank you very much in advance!

Cheers,
Fabian

 

[Georg Brandl]

I recently started learning C since I want to be able to write Python
extension modules. In fact, there is no need for it, but I simply want
to try something new ...

I tried to implement the bubblesort algorithm in C and to use it in
python; bubblesort.c compiles fine, but whenever I want to import the
modul and call the function I get a segmentation fault. This is what the
code looks like:

static PyObject *py_bubblesort(PyObject *self, PyObject *args) {
PyObject *seq = NULL, *item, *newseq = NULL;
int seqlen, i;

long it;

if(!PyArg_ParseTuple(args, "O", &seq)) {
return NULL;
}
seq = PySequence_Fast(seq, "argument must be iterable");
if(!seq) {
return NULL;
}
seqlen = PySequence_Fast_GET_SIZE(seq);
int list[seqlen];
for (i = 0; i <= seqlen; i++) {

That is one iteration too much. Use

for (i = 0; i < seglen; i++)

item = PySequence_Fast_GET_ITEM(seq, i);

Now item is a PyObject*. You'll have to convert it to an integer now:

it = PyInt_AsLong(item);
if (it == -1 && PyErr_Occurred()) {
Py_DECREF(seq);
/* set a new exception here if you like */
return NULL;
}

list = it;
}
bubblesort(list, seqlen);
newseq = PyList_New(seqlen);
if(!newseq) {

Do not forget to DECREF seq:
Py_DECREF(seq);

return NULL;
}
for(i = 0; i < seqlen; i++) {
PyList_SetItem(newseq, i, list);

List items must be PyObject*s, not plain ints. Use:
PyList_SetItem(newseq, i, PyInt_FromLong(list));
(This is sloppy error checking, but if PyInt_FromLong fails you're out of
memory anyways

}

Again, seq is not needed anymore:
Py_DECREF(seq);

return newseq;

bubblesort(int list[], int seqlen) is doing the actual job and it is
working.

What did I do wrong? As I am quite new to C, I probably made many
mistakes, so please feel free to correct me.

There's quite a bit you can overlook, especially stale references to PyObjects.
I'm not even sure the code compiles or runs correctly with my corrections

Cheers,
Georg

 

[Fabian Steiner]

[...]
for (i = 0; i <= seqlen; i++) {

That is one iteration too much. Use

for (i = 0; i < seglen; i++)

item = PySequence_Fast_GET_ITEM(seq, i);

Now item is a PyObject*. You'll have to convert it to an integer now:

it = PyInt_AsLong(item);

Why do you use PyInt_AsLong() here? As the documentation says it returns
a long type: long PyInt_AsLong(PyObject *io)
On the other hand I can't find anything like PyInt_AsInt().

if (it == -1 && PyErr_Occurred()) {
Py_DECREF(seq);

Why is this Py_DECREF() needed? What does it do exactly? When do I have
to call this function? Obviously, there is also Py_INCREF(). When do you
need this function?

[...]
There's quite a bit you can overlook, especially stale references to PyObjects.
I'm not even sure the code compiles or runs correctly with my corrections

Now, it compiles fine, without any warnings and using it in Python works
either Now I have to try to understand what the different parts are
doing and why they are necessary.

Thank you very much so far!

Cheers,
Fabian

 

[Georg Brandl]

[...]
for (i = 0; i <= seqlen; i++) {

That is one iteration too much. Use

for (i = 0; i < seglen; i++)

item = PySequence_Fast_GET_ITEM(seq, i);

Now item is a PyObject*. You'll have to convert it to an integer now:

it = PyInt_AsLong(item);

Why do you use PyInt_AsLong() here? As the documentation says it returns
a long type: long PyInt_AsLong(PyObject *io)
On the other hand I can't find anything like PyInt_AsInt().

Python's int objects carry a long, so they can return you this long.
On most 32-bit platforms, int == long anyway, but for 64-bit you'd have
to declare list as long too.

Why is this Py_DECREF() needed? What does it do exactly? When do I have
to call this function? Obviously, there is also Py_INCREF(). When do you
need this function?

This is for reference counting. Since you created seq with PySequence_Fast
you "own" a reference to it (it has reference count 1). Since nothing else
references that sequence, it has to be deallocated before the function exits.
You use Py_DECREF to decrease the reference count to 0, thus telling Python
that it's safe to free the memory associated with it.

Most API functions that return a PyObject increase its reference count by 1,
leaving you in charge to do something with this ("your") reference.

Other examples of how references can be juggled with:

item = PySequence_Fast_GET_ITEM(seq, i);

PySequence_Fast_GET_ITEM does return a PyObject, but it doesn't increase the
reference count, so you don't have to DECREF item anywhere. However, if you were
to store item in a structure of some sort, you'd have to INCREF it so that
Python doesn't destroy it while it's still referenced by your structure.

PyList_SetItem(newseq, i, PyInt_FromLong(list));

PyInt_FromLong() returns a new PyObject with one reference, but PyList_SetItem
"steals" that reference from you (it stores the PyObject in a structure without
increasing its reference count). Therefore, you don't own a reference to the
integer object anymore and don't have to DECREF it.

newseq = PyList_New(seqlen);
(...)
return newseq;

Here, you own a reference to newseq, but you return the object, so you have to
keep it alive. Thus, no DECREF.

[...]
There's quite a bit you can overlook, especially stale references to PyObjects.
I'm not even sure the code compiles or runs correctly with my corrections

Now, it compiles fine, without any warnings and using it in Python works
either Now I have to try to understand what the different parts are
doing and why they are necessary.

Cheers,
Georg

 

[baalbek]

What do I have to change in order to make the code work?

I'm afraid to say: your knowledge of C

But don't worry, C is an easy language to learn, and a very valuable
skill to have.

Baalbek