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 <[email protected]>wrote:

> On Mon, Aug 12, 2013 at 12:28 PM, Neel Patel
> <[email protected]> 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
>

Attachment: xmlParser_V1.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Reply via email to