Re: [bug-recutils] Python support

[Michał Masłowski]

Hi.

> Thanks, I applied the patch, but I am still getting the memory leak
> when I run the sample.c program (which I sent).

It works here at commit a55a649723cd06c62fd103d3b9e0cd57fe83b731.

> I even installed the software files at a particular location
> to make sure it wasn't using another version. I am still unable to get
> line numbers in recutils files during debug. So I could not exactly
> find which line is causing the leak. Only the function name,
> rec_parse_db() is shown.

Run "ldd sample" to check what shared libraries it uses, maybe there is
a system build of librec.  LD_LIBRARY_PATH and LD_LIBRARY_PRELOAD should
help force the use of another library.

> I have written bindings for some modify functions, such as
> insert_rset() in DB (I have not uploaded it). It is inserting fine,
> but I get the same double free memory error.
> I have also written a binding for rec_db_query() function, but it does
> not work. It throws an "undefined symbol" error when I call the C
> recdb query function. Why is that so?

Seeing the code would help.  Shared library function symbols are
resolved at first call, it's very possible to get such an error at
runtime unless maybe using -Wl,--no-undefined.

> My loadfile() function loads a file into a DB by erasing the previous
> data in it. This is because the C parser function does not append the
> parsed data in the DB, but overwrites it. Is this how it should be? At
> present we can add more data from a file to a DB by parsing the file
> into another DB, getting an rset, and then inserting the rset into the
> DB (this works for me, except for the memory error at the  end).

See utils/recutl.c, recutl_parse_db_from_file: rec_parse_db is not used
there.  Only recinf and tests use rec_parse_db, other utils support
loading databases from multiple files.

If the bindings had separate C and Python modules, it could be useful to
implement recutl_parse_db_from_file in the Python part (just like librec
and recutl are separate).  This would need an API for using parsers with
Python file objects, I think it should be possible with PyFile_AsFile.

I have two other comments related to this code:

+  if(in == NULL)
+    {
+       PyErr_SetString(PyExc_IOError, "File not found, or disk full");
+      return NULL;
+    }

doesn't mention many other possible errors (C code would in this case
read errno), using a Python file object instead of the filename in the
loadfile API would solve this.

+  if (self->rdb == NULL)
+    {
+        PyErr_SetString(PyExc_AttributeError, "DB is empty");
+        return NULL;
+    }

I think there is no need to support rdb == NULL, it could allow removing
many conditionals for this and other objects.  Empty databases have no
record sets and rdb != NULL.

pgpOmGk3SeTPK.pgp
Description: PGP signature