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]

Re: C-API: A beginner's problem

Fabian Steiner wrote:[color=blue]
> What did I do wrong? As I am quite new to C, I probably made many
> mistakes, so please feel free to correct me.[/color]

The following line:
[color=blue]
> for (i = 0; i <= seqlen; i++) {[/color]

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

[Heikki Salo]

Re: C-API: A beginner's problem

Heikki Salo wrote:[color=blue]
> Fabian Steiner wrote:[color=green]
>> What did I do wrong? As I am quite new to C, I probably made many
>> mistakes, so please feel free to correct me.[/color]
>
> The following line:
>[color=green]
> > for (i = 0; i <= seqlen; i++) {[/color]
>
> Should be "for (i = 0; i < seqlen; i++) {". Otherwise the last
> assignment will be out of bounds and probably corrupts heap.[/color]

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]

Re: C-API: A beginner's problem

Heikki Salo wrote:
[color=blue]
>
> 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.[/color]

.... 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]

Re: C-API: A beginner's problem

Duncan Booth wrote:[color=blue]
> Heikki Salo wrote:
>[color=green]
> >
> > 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.[/color]
>
> ... 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).[/color]

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

[Duncan Booth]

Re: C-API: A beginner's problem

Nick Smallbone wrote:
[color=blue]
> Duncan Booth wrote:[color=green]
>> Heikki Salo wrote:
>>[color=darkred]
>> >
>> > 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.[/color]
>>
>> ... 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).[/color]
>
> They are allowed in C99, according to GCC's manual.
>
>[/color]

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

[Fabian Steiner]

Re: C-API: A beginner's problem

Heikki Salo wrote:[color=blue]
> Heikki Salo wrote:[color=green]
>> Fabian Steiner wrote:[color=darkred]
>>> What did I do wrong? As I am quite new to C, I probably made many
>>> mistakes, so please feel free to correct me.[/color]
>>
>> The following line:
>>[color=darkred]
>> > for (i = 0; i <= seqlen; i++) {[/color]
>>
>> Should be "for (i = 0; i < seqlen; i++) {". Otherwise the last
>> assignment will be out of bounds and probably corrupts heap.[/color]
>
> 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.[/color]

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]

Re: C-API: A beginner's problem

Fabian Steiner wrote:[color=blue]
> 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;[/color]

long it;
[color=blue]
> 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++) {[/color]

That is one iteration too much. Use

for (i = 0; i < seglen; i++)
[color=blue]
> item = PySequence_Fast _GET_ITEM(seq, i);[/color]

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;
}
[color=blue]
> list[i] = it;
> }
> bubblesort(list , seqlen);
> newseq = PyList_New(seql en);
> if(!newseq) {[/color]

Do not forget to DECREF seq:
Py_DECREF(seq);
[color=blue]
> return NULL;
> }
> for(i = 0; i < seqlen; i++) {
> PyList_SetItem( newseq, i, list[i]);[/color]

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 ;)
[color=blue]
> }[/color]

Again, seq is not needed anymore:
Py_DECREF(seq);
[color=blue]
> 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.[/color]

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]

Re: C-API: A beginner's problem

Georg Brandl wrote:[color=blue]
> Fabian Steiner wrote:[color=green]
>> [...]
>> for (i = 0; i <= seqlen; i++) {[/color]
>
> That is one iteration too much. Use
>
> for (i = 0; i < seglen; i++)
>[color=green]
>> item = PySequence_Fast _GET_ITEM(seq, i);[/color]
>
> Now item is a PyObject*. You'll have to convert it to an integer now:
>
> it = PyInt_AsLong(it em);[/color]

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().
[color=blue]
> if (it == -1 && PyErr_Occurred( )) {
> Py_DECREF(seq);[/color]

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?
[color=blue]
> [...]
> 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 ;)[/color]

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]

Re: C-API: A beginner's problem

Fabian Steiner wrote:[color=blue]
> Georg Brandl wrote:[color=green]
>> Fabian Steiner wrote:[color=darkred]
>>> [...]
>>> for (i = 0; i <= seqlen; i++) {[/color]
>>
>> That is one iteration too much. Use
>>
>> for (i = 0; i < seglen; i++)
>>[color=darkred]
>>> item = PySequence_Fast _GET_ITEM(seq, i);[/color]
>>
>> Now item is a PyObject*. You'll have to convert it to an integer now:
>>
>> it = PyInt_AsLong(it em);[/color]
>
> 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().[/color]

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.
[color=blue][color=green]
>> if (it == -1 && PyErr_Occurred( )) {
>> Py_DECREF(seq);[/color]
>
> 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?[/color]

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.
[color=blue][color=green]
>> [...]
>> 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 ;)[/color]
>
> 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.[/color]

Cheers,
Georg

[baalbek]

Re: C-API: A beginner's problem

Fabian Steiner wrote:
[color=blue]
> What do I have to change in order to make the code work?[/color]

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