yangqiaosheng / libcitygml

Automatically exported from code.google.com/p/libcitygml
GNU Lesser General Public License v2.1
0 stars 0 forks source link

libxml2 parsing: Crash during xmlFreeParserCtxt( context ); #6

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Compile with libxml2 (Linux, Ubuntu Lucid), Run citygmltest with any file
2. Crash will happen in xmlFreeParserCtxt( context ); 

What is the expected output? What do you see instead?
No crash !

What version of the product are you using? On what operating system?
Trunk as of 15th September 2010

Please provide any additional information below.
xmlSAXHandler needs to be allocated on the Heap so that
the xmlFreeParserCtxt function can free it afterwards.

See patch below for a fix.

Original issue reported on code.google.com by wbaer76@googlemail.com on 15 Sep 2010 at 7:11

Attachments:

GoogleCodeExporter commented 9 years ago
Hi wbaer76!
Thanks for the report.
Could you try to comment these lines : (l.146 & 196)  "context->sax = &sh;"

Actually, these instructions are useless because libxml has already assigned 
the sax member when the method xmlCreatePushParserCtxt was called. Moreover, it 
breaks the context integrity because Libxml creates a copy of sh during 
xmlCreatePushParserCtxt call. So commenting this line should be enough.

Could you tell if it worked ? In the case it doesn't, we may integrate your 
patch :)

Thank you for your help!

Manuel

Original comment by garnier....@gmail.com on 15 Sep 2010 at 4:50

GoogleCodeExporter commented 9 years ago
It works for the push parser. I had to enable using this load method in 
citygmltest (see patch) and to fix an argument (filter -> params).

For the other load method (xmlCreateFileParserCtxt) it does not work as there 
is no
xmlSAXHandlerPtr parameter in the constructor. So it must be set afterwards, but
as dynamic allocated object as I reported in the first comment.

Why are there two load methods? It seems the xmlCreateFileParserCtxt method is 
an
internal method not really intended to be used by applications.

I propose to redirect the second load method based on filename to the first 
based
on a stream. See attached patch2.diff. Both load methods work now on Ubuntu.

Wolfgang

Original comment by wbaer76@googlemail.com on 16 Sep 2010 at 6:23

Attachments:

GoogleCodeExporter commented 9 years ago
Yes, that's right for xmlCreateFileParserCtxt, no saxHandler is given.
Sure, it is an internal method, but it allows a lower level dev.
There is plenty of methods to get the work done, so I'll look at it in a few 
hours, when I have an available ubuntu machine.

Regards,

Manuel

Original comment by garnier....@gmail.com on 16 Sep 2010 at 12:58

GoogleCodeExporter commented 9 years ago
Hi!

The bug is fixed, you can try now, it should work.
Not tested on windows though, but I think it should be okay.

Thanks again for the report !

Regards,
Manuel

Original comment by garnier....@gmail.com on 16 Sep 2010 at 9:17

GoogleCodeExporter commented 9 years ago
Issue 9 has been merged into this issue.

Original comment by garnier....@gmail.com on 22 Sep 2010 at 7:52