Re: [patch] fixing segfault because of empty coord cache

[Stefan Schimanski]

Critical bugs don't get less critical by ignorance.

If anybody wants to know more:





Stefan


Index: src/mathed/CommandInset.h
===================================================================
--- src/mathed/CommandInset.h   (Revision 18719)
+++ src/mathed/CommandInset.h   (Arbeitskopie)
@@ -40,6 +40,9 @@
        virtual docstring const screenLabel() const;
        ///
        docstring const & commandname() const { return name_; }
+       ///
+       bool isActive() const { return false; }
+
private:
        virtual std::auto_ptr<Inset> doClone() const;
Index: src/mathed/InsetMathNest.cpp
===================================================================
--- src/mathed/InsetMathNest.cpp        (Revision 18719)
+++ src/mathed/InsetMathNest.cpp        (Arbeitskopie)
@@ -732,7 +732,7 @@
                    && cur.inMacroMode() && cur.macroName() != "\\"
                    && cur.macroModeClose()) {
                        MathAtom const atom = cur.prevAtom();
-                       if (atom->asNestInset() && atom->nargs() > 0) {
+                       if (atom->asNestInset() && atom->isActive()) {
                                cur.posLeft();
                                cur.pushLeft(*cur.nextInset());
                        }

1) CommandInset (used e.g. for references in mathed) has two cells to hold information about where it points to. But it only draws a button, not the cells themselves. So accessing the cells' coordinates in the cache during drawSelection segfaults. 2) This all happens only because, after creating the InsetMathRef (derived from CommandInset) with \ref, the cursor is inside of it. Hence, InsetMathNest will try to draw the selection inside the InsetMathRef below the cells. 3) So, why is the cursor inside it? Because it is derived from InsetMathNest and has >= 1 cells. The LFUN_SELF_INSERT handler of the hull (in fact in InsetMathNest::doDispatch) takes this as a reason to put the cursor inside. I guess that's not correct, because the cursor- left behavior normally checks the isActive() method before entering an inset. But here isActive() is not called. So I propose the following patch (which fixes #3715). The first hunk is trivial. But I am not sure about the consequences of the second. I think it's the correct way to insert an inset, but maybe there are other inset which depend on the old (wrong) behavior. Comments?

Am 04.06.2007 um 23:35 schrieb Jean-Marc Lasgouttes:

"Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes:

Stefan> Hmm, it's a not very complex patch for a critical bug about a
Stefan> segfault and nobody has a comment?

Actually I am wondering why suddenly we have these crashes in very
simple operations (like breakParagraph this one). Isn't there
something else hidden?

Stefan

This one has nothing to do with breakParagraph. It is IMO just non- defensive programming which strikes back. The InsetMathNest just assumes that all its cells are drawn (and hence are in the coordinate cache). But this assumption shouldn't be made. If it is made, we have to document it and check the derived classes. I am pretty sure that someof them warm up the cache of all cells as a consequence of patchworking against crashes in other InsetMathMethods method (e.g. editXY).

PGP.sig
Description: Signierter Teil der Nachricht