Re: Possible crash fix for xmlCleanupParser

Hi Dave,

Thanks for the comments.

Find the updated patch after fixing the below review comments.
-- Removed the cleanup call when we exit the application.

Thanks,
Neel Patel

On Mon, Aug 12, 2013 at 5:58 PM, Dave Page <dave(dot)page(at)enterprisedb(dot)com>wrote:

> On Mon, Aug 12, 2013 at 12:28 PM, Neel Patel
> <neel(dot)patel(at)enterprisedb(dot)com> wrote:
> > Hi Dave,
> >
> > Please find attached patch for the below issue in pgAdmin.
> >
> > "As per libxml2 document (below link) we should call xmlCleanupParser()
> only
> > one time when the application exists not every time when we compute the
> xml
> > parser. As per the document application may crash if another thread or
> > plugin still using the libxml2".
> >
> > http://xmlsoft.org/html/libxml-parser.html#xmlCleanupParser
>
> For the benefit of the list archives and readers, this fixes an issue
> in an EDB product that's derived from pgAdmin. The crash we have isn't
> present in pgAdmin at the moment, but because we're mis-using a
> libxml2 API, it could be in the future.
>
> > Kindly review it and let me know for any modification.
>
> Is there any need to cleanup when we're exiting anyway? Memory will be
> freed then. Unless there's some reason I'm unaware of, we should just
> remove the call from frmReport.
>
> A couple of minor stylistic gripes:
>
> - When you have a comment section of code, it's almost always a
> logical block, even if it's just the comment and one line. This should
> be illustrated with blank lines before and after, i.e. instead of:
>
> wxWindow *fr;
> windowList::Node *node;
> // Clean up the memory allocated by the library ( libxml2 )
> xmlCleanupParser();
> while ((node = frames.GetFirst()) != NULL)
> {
>
> You should use:
>
> wxWindow *fr;
> windowList::Node *node;
>
> // Clean up the memory allocated by the library ( libxml2 )
> xmlCleanupParser();
>
> while ((node = frames.GetFirst()) != NULL)
> {
>
> - Try to make your comments short, and to the point. In this case why
> not just name the library instead of putting it in braces after what
> on it's own is an unhelpful comment? e.g.
>
> // Cleanup the memory allocated by libxml2.
>
> --
> Dave Page
> Chief Architect, Tools & Installers
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>