Feature request: error message reporting to a function

[GoogleCodeExporter]

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 reported on code.google.com by davedyks...@gmail.com on 4 Mar 2013 at 10:42

[GoogleCodeExporter]

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

Original comment by davedyks...@gmail.com on 5 Mar 2013 at 10:26

Attachments:

• pacparser-bufprint.patch

[GoogleCodeExporter]

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

Original comment by davedyks...@gmail.com on 27 Mar 2013 at 9:53

[GoogleCodeExporter]

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?

Original comment by davedyks...@gmail.com on 27 Mar 2013 at 9:57

[GoogleCodeExporter]

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

Original comment by manugarg on 29 Mar 2013 at 8:02

• Changed state: Accepted

[GoogleCodeExporter]

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

Original comment by davedyks...@gmail.com on 29 Mar 2013 at 8:24

[GoogleCodeExporter]

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=c54b2db142591650846e9b8359f4
e6b6d5081356

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.

Original comment by manugarg on 4 Apr 2013 at 6:21

• Changed state: Started

[GoogleCodeExporter]

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

Original comment by davedyks...@gmail.com on 5 Apr 2013 at 1:03

[GoogleCodeExporter]

Hey Dave,

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

Manu

Original comment by manugarg on 5 Apr 2013 at 5:50

[GoogleCodeExporter]

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.

Original comment by manugarg on 15 Apr 2013 at 6:02

• Changed state: Fixed

[GoogleCodeExporter]

Thanks, Manu, looks great.

Original comment by davedyks...@gmail.com on 15 Apr 2013 at 9:56