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_Parse Tuple(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[i] = item;
}
bubblesort(list , seqlen);
newseq = PyList_New(seql en);
if(!newseq) {
return NULL;
}
for(i = 0; i < seqlen; i++) {
PyList_SetItem( newseq, i, list[i]);
}

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]

Fabian Steiner wrote:

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]

Heikki Salo wrote:

Fabian Steiner wrote:

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.

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

[Duncan Booth]

Heikki Salo wrote:

And closer look tells that the code should not even compile. Is the
code cut & pasted directly? Line "list[i] = 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]

Duncan Booth wrote:

Heikki Salo wrote:

And closer look tells that the code should not even compile. Is the
code cut & pasted directly? Line "list[i] = 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]

Nick Smallbone wrote:

Duncan Booth wrote:

Heikki Salo wrote:

>
> And closer look tells that the code should not even compile. Is the
> code cut & pasted directly? Line "list[i] = 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]

Heikki Salo wrote:

Heikki Salo wrote:

Fabian Steiner wrote:

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.

And closer look tells that the code should not even compile. Is the code
cut & pasted directly? Line "list[i] = 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]

Fabian Steiner wrote:

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_Parse Tuple(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(it em);
if (it == -1 && PyErr_Occurred( )) {
Py_DECREF(seq);
/* set a new exception here if you like */
return NULL;
}
list[i] = it;
}
bubblesort(list , seqlen);
newseq = PyList_New(seql en);
if(!newseq) {
Do not forget to DECREF seq:
Py_DECREF(seq);
return NULL;
}
for(i = 0; i < seqlen; i++) {
PyList_SetItem( newseq, i, list[i]);
List items must be PyObject*s, not plain ints. Use:
PyList_SetItem( newseq, i, PyInt_FromLong( list[i]));
(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]

Georg Brandl wrote:

Fabian Steiner wrote:

[...]
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(it em);

Why do you use PyInt_AsLong() here? As the documentation says it returns
a long type: long PyInt_AsLong(Py Object *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]

Fabian Steiner wrote:

Georg Brandl wrote:

Fabian Steiner wrote:

[...]
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(it em);

Why do you use PyInt_AsLong() here? As the documentation says it returns
a long type: long PyInt_AsLong(Py Object *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.

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?

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[i]));

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(seql en);
(...)
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