Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > Python > C-API: A beginner's problem

Reply
Thread Tools

C-API: A beginner's problem

 
 
Fabian Steiner
Guest
Posts: n/a
 
      03-19-2006
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[i] = item;
}
bubblesort(list, seqlen);
newseq = PyList_New(seqlen);
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
 
Reply With Quote
 
 
 
 
Heikki Salo
Guest
Posts: n/a
 
      03-19-2006
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.
 
Reply With Quote
 
 
 
 
Heikki Salo
Guest
Posts: n/a
 
      03-19-2006
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.
 
Reply With Quote
 
Duncan Booth
Guest
Posts: n/a
 
      03-19-2006
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).
 
Reply With Quote
 
Nick Smallbone
Guest
Posts: n/a
 
      03-19-2006
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.

 
Reply With Quote
 
Duncan Booth
Guest
Posts: n/a
 
      03-19-2006
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.
 
Reply With Quote
 
Fabian Steiner
Guest
Posts: n/a
 
      03-19-2006
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
 
Reply With Quote
 
Georg Brandl
Guest
Posts: n/a
 
      03-19-2006
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_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[i] = 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[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
 
Reply With Quote
 
Fabian Steiner
Guest
Posts: n/a
 
      03-20-2006
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(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
 
Reply With Quote
 
Georg Brandl
Guest
Posts: n/a
 
      03-20-2006
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(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.

>> 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(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
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Problem problem problem :( Need Help Mike ASP General 2 05-11-2004 08:36 AM



Advertisments