lucy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marvin Humphrey <mar...@rectangular.com>
Subject [lucy-dev] Error reporting in allot_params
Date Fri, 07 Jan 2011 03:31:41 GMT
Greets,

The main function we use for converting named arguments in Perl-space to
variables in C-space is XSBind_allot_params(), located in
trunk/perl/xs/XSBind.c.  For the Python people out there, allot_params()
vaguely resembles the PyArg_Parse* functions documented at
<http://docs.python.org/py3k/c-api/arg.html#api-functions> and
<http://docs.python.org/py3k/extending/extending.html>.  (There is no analogue
in the Perl C API.)

There are two improvements I have in mind for allot_params().  The first has
to do with error reporting.

If allot_params() discovers one of the following problems, it throws an
exception:

    * Unrecognized parameter name.
    * Odd number of arguments (i.e. not key-value pairs).
    * Missing parameter name hash.

However, the resulting stack trace does not include the name of the method
where the error occurred.  It mentions allot_params(), but it does not mention
the XS binding caller:

    Invalid parameter: 'bogus'
        cfish_XSBind_allot_params at xs/XSBind.c line 408
        at foo.t line 5

If we change allot_params() to return false and store an exception object in
Err_error on failure, then the calling code can RETHROW the exception...

        bool_t args_ok = XSBind_allot_params( &(ST(0)), 1, 
            items, "Lucy::Search::IndexSearcher::new_PARAMS", 
            &index_sv, "index", 5,
            NULL);
        if (!args_ok) {
            RETHROW(INCREF(Err_get_error()));
        }   

... so that we see its location in the stack trace instead.

    Invalid parameter: 'bogus'
        cfish_XSBind_allot_params at xs/XSBind.c line 408
        XS_Lucy_Search_IndexSearcher_new at lib/Lucy.xs line 1647
        at foo.t line 5

"XS_Lucy_Search_IndexSearcher_new" isn't quite as good as
"Lucy::Search::IndexSearcher::new", but it's better than nothing.

The second improvement I have in mind is to give allot_params() significantly
more responsibility with regards to extracting values.  I'll explain the
details in a separate email.

Marvin Humphrey


Mime
View raw message