Feature request: error message reporting to a function

[manugarg]

From [email protected] on March 04, 2013 14:42:01

It's rather nasty for libraries to send error messages to stderr because different applications do different things with error messages, for example many applications send them to syslog.  The underlying JavaScript library handles that with JS_SetErrorReporter to supply a callback function.  An alternate method some libraries use is to have an error reporting function (for example dlerror()) that the caller can use to request error messages after some other API function returns an error.  Either one works.

As it stands now, it looks like I'm going to have to patch pacparser.[ch] to add this or turn stderr into a pipe and catch messages that way.  The latter is too disgusting so I'll probably do the former.

Original issue: http://code.google.com/p/pacparser/issues/detail?id=23

[manugarg]

From [email protected] on March 05, 2013 14:26:24

Attaching patch including fixes for issue #22 and issue #24

[manugarg]

From [email protected] on March 27, 2013 14:53:46

If it helps, I have now put my patches into a git clone of pacparser: https://code.google.com/r/davedykstra-pacparser-patches

[manugarg]

From [email protected] on March 27, 2013 14:57:44

I tried to generate a man page for the new API function, and I noticed that the man pages for the others were generated with c2man.  I tried to google for a version of c2man, and the one I found from http://www.ciselant.de/c2man/c2man.html hasn't been updated since 2000 and doesn't seem to be able to handle the typedef syntax I needed to use to define a pointer to a callback function.  Is this a serious obstacle?

[manugarg]

From manugarg on March 29, 2013 13:02:46

Hi Dave,

I am trying to integrate your patches. I had a question about the use of errorvprinter_context. Since this variable is only used by errorvprinter_func, will it not make more sense for the definition of errorvprinter_func (as defined by the user of the library) to subsume this variable. I am a little uncomfortable in letting this variable be part of the pacparser code without understanding its significance and what it might contain.

Does that make sense?

Thanks again for the patches.

Cheers,
Manu

Status: Accepted

[manugarg]

From [email protected] on March 29, 2013 13:24:20

Hi Manu,

Maybe.  I do think it is pretty typical for library callback functions to include a parameter that applications can use to put in any pointer they want, though.  That way if the application has multiple objects it is dealing with it can pass a relevant object through, especially if it is trying to be thread safe.  See for example the printing callback that comes back from javascript to your print_jserror function.  On the other hand, the pacparser api isn't threadsafe anyway and doesn't include it's own context object, so it probably doesn't help much to have a context here.  I see now that the print_jserror function is given the javascript context, not a context made up by the user of the library, although the user of the library could in theory extend the object with more data.  I don't see what harm it can cause to have it there, but if you'd rather not have the context parameter to the callback function I'm ok with taking it out.  I am using it in my application to pass a pointer to a buffer that's on the stack, but I could always put a pointer to it in a static variable instead.

Dave

[manugarg]

From manugarg on April 03, 2013 23:21:19

Hi Dave,

I have committed the change on your behalf (with you as author). Please take a look to make sure that I have missed anything: https://code.google.com/p/pacparser/source/detail?r=c54b2db142591650846e9b8359f4e6b6d5081356 I have dropped the context variable in error reporting function as I reported earlier. There was no harm in keeping it except that it was a little confusing here.

Status: Started

[manugarg]

From [email protected] on April 04, 2013 18:03:58

Manu,

Looks great, thanks a lot.  I integrated it with my application and it works.

Now the remaining question is the man page.  I suppose the easiest thing short of finding another tool is to first hack a copy of pacparser.h to comment out the part that the tool can't handle, then manually edit the pacparser_set_error_printer.

Dave

[manugarg]

From manugarg on April 04, 2013 22:50:44

Hey Dave,

I am working on switching our documentation to doxygen. Doxygen has many more features and is actively maintained.

Manu

[manugarg]

From manugarg on April 14, 2013 23:02:01

Documentation has been fixed as well. I'll mark this bug as 'Fixed'.

Requisite changes are already in main repository. I'll soon cut a new release.

Status: Fixed

[manugarg]

From [email protected] on April 15, 2013 14:56:43

Thanks, Manu, looks great.