bug-recutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug-recutils] Python support


From: Maninya M
Subject: Re: [bug-recutils] Python support
Date: Tue, 3 Sep 2013 17:25:31 +0530

Hi,
Please check python branch. The file "pyrec.py" is the python module that
helps in exception handling. I have not written function in this file
for all the extension functions.
Should I? At present, some functions are called through this python
module (import pyrec), and some
directly from the extension functions(import recutils).

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

I have got the functions working, it was because of the system build of librec.
I got it to use my installed library and it all works.

>> 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 written a pyappendfile function that works similar to the
recutl_parse_db_from_file.
I have done the file object error checking in C itself, because it
would be easier to read the
errno value. I have created a new exception object for the binding.
The file opening is done in the
C part, so we only need to pass the filename to the python function.
Please see recutilstest.py.

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

Yes, I am reading the value of errno using strerror in the C code.
I think this would be a better approach?

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

I have removed this.


-- 
Maninya



reply via email to

[Prev in Thread] Current Thread [Next in Thread]