Thanks, applied.
On Mon, Aug 12, 2013 at 1:43 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> 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.page@enterprisedb.com>
> wrote:
>>
>> On Mon, Aug 12, 2013 at 12:28 PM, Neel Patel
>> <neel.patel@enterprisedb.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
>
>
--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
