On Feb 3, 2008 5:39 AM, Niko Kiirala <niko@...1267...> wrote:
Sat, 2 Feb 2008 09:11:51 -0800 "John Faith" <jfaith7@...400...> kirjoitti:
I found that this small change prevents a startup crash on OS X 10.3.9 (also added to https://bugs.launchpad.net/inkscape/+bug/185689):
--- inkscape.trunk/src/sp-filter.cpp.orig Sat Dec 22 12:15:36 2007 +++ inkscape.trunk/src/sp-filter.cpp.noStartupCrash Sat Feb 2 08:55:03 2008 @@ -110,7 +110,7 @@ sp_filter_init(SPFilter *filter)
filter->_renderer = NULL;
- filter->_image_name = map<gchar *, int, ltstr>();
filter->_image_name.clear();
filter->filterRes = NumberOptNumber();
While I'm happy that Inkscape can actually run, it's not obvious to me why this fixes the crash, nor if this is generaly an acceptable thing to do. Could someone possibly test that this does not cause a problem on another platform?
By the way, after applying this patch, I have another crash in sp-filter.cpp when opening "Help->About Inkscape", so I wonder if there's either a std::map problem with gcc 3.3 or memory is getting trashed. Random theories welcome :-).
That was one of the most annoying bugs I've ever had to debug. SPFilter is not an actual C++ class, so it's not constructed. This means, that filter->_image_name is not construced either. Before that line you changed, filter->_image_name has rubbish state and that line is trying to set it to sensible state.
So... That line should just call the constructor for filter->_image_name. I can see, how my original version may be compiled as 'construct a temporary map and use operator= to copy it to filter->_image_name'.
Hi Niko, Thanks for the reply.
I wonder how the filter struct memory is allocated?
As an experiment, I added 2 vars in sp_filter_init():
static SPFilter* filterNew = new SPFilter(); static SPFilter* filterAlloc = (SPFilter*)malloc(sizeof(SPFilter));
, and in gdb saw: (gdb) print filter->_image_name._M_t._M_header $7 = (_Rb_tree_node<std::pair<char* const, int> > *) 0x0 (gdb) print filterNew->_image_name._M_t._M_header $9 = (_Rb_tree_node<std::pair<char* const, int> > *) 0x71a1fe0 (gdb) print filterAlloc->_image_name._M_t._M_header $8 = (_Rb_tree_node<std::pair<char* const, int> > *) 0x0
, which leads me to believe that the filter->_image_name member is not being initialized correctly.
I found that changing the SPFilter struct's _image_name from a std::map to a pointer to a std::map and using 'filter->_image_name = new std::map<gchar *, int, ltstr>;' in sp_filter_init() solves my crashing problems; I can now startup and view the About dialog.
, John
Sun, 3 Feb 2008 21:03:03 -0800 "John Faith" <jfaith7@...400...> kirjoitti:
I found that changing the SPFilter struct's _image_name from a std::map to a pointer to a std::map and using 'filter->_image_name = new std::map<gchar *, int, ltstr>;' in sp_filter_init() solves my crashing problems; I can now startup and view the About dialog.
I'm not so much into using a pointer there, but yes, that is a good solution. 'new' will allocate the memory for the map and run the constructor in proper order.
Please attach your patch for this, so we can get this fixed in SVN.
On 2008-February-04 , at 08:42 , Niko Kiirala wrote:
Sun, 3 Feb 2008 21:03:03 -0800 "John Faith" <jfaith7@...400...> kirjoitti:
I found that changing the SPFilter struct's _image_name from a std::map to a pointer to a std::map and using 'filter->_image_name = new std::map<gchar *, int, ltstr>;' in sp_filter_init() solves my crashing problems; I can now startup and view the About dialog.
I'm not so much into using a pointer there, but yes, that is a good solution. 'new' will allocate the memory for the map and run the constructor in proper order.
Please attach your patch for this, so we can get this fixed in SVN.
Given the amount of carefully thought work John has been doing for a while already, I think he should get SVN access. He's well beyond the two patches "requirement". It would speed his contributions up and Inkscape will benefit much from it.
JiHO --- http://jo.irisson.free.fr/
On Feb 3, 2008 11:42 PM, Niko Kiirala <niko@...1267...> wrote:
Sun, 3 Feb 2008 21:03:03 -0800 "John Faith" <jfaith7@...400...> kirjoitti:
I found that changing the SPFilter struct's _image_name from a std::map to a pointer to a std::map and using 'filter->_image_name = new std::map<gchar *, int, ltstr>;' in sp_filter_init() solves my crashing problems; I can now startup and view the About dialog.
I'm not so much into using a pointer there, but yes, that is a good solution. 'new' will allocate the memory for the map and run the constructor in proper order.
Please attach your patch for this, so we can get this fixed in SVN.
Done. Attached to https://bugs.launchpad.net/inkscape/+bug/185689 and I did a fresh 'svn co', applied the patch and it still compiles and runs.
The one thing I'm not sure about is the destruction of the SPFilter*. I added a 'delete filter->_image_name;' in sp_filter_release(). Is this the right place? If not, I'll redo the patch.
, John
Mon, 4 Feb 2008 21:10:07 -0800 "John Faith" <jfaith7@...400...> kirjoitti:
Done. Attached to https://bugs.launchpad.net/inkscape/+bug/185689 and I did a fresh 'svn co', applied the patch and it still compiles and runs.
Thank you for the patch. I've committed it in SVN.
The one thing I'm not sure about is the destruction of the SPFilter*. I added a 'delete filter->_image_name;' in sp_filter_release(). Is this the right place? If not, I'll redo the patch.
Yes, that is the right place.
participants (3)
-
jiho
-
John Faith
-
Niko Kiirala