Bug #781893 (Crash after moving a Bezier node after Knot path effect)

Although issue not solved, prevent Inkscape from crashing. Add some code to make code more robust.

(bzr r11547)

6 files changed, 117 insertions, 57 deletions

diff --git a/src/event-context.cpp b/src/event-context.cpp
index e83f6a209..3c0f369cb 100644
--- a/src/event-context.cpp
+++ b/src/event-context.cpp
@@ -177,7 +177,7 @@ static void sp_event_context_dispose(GObject *object) {
 void sp_event_context_update_cursor(SPEventContext *ec) {
     GtkWidget *w = GTK_WIDGET(sp_desktop_canvas(ec->desktop));
     if (gtk_widget_get_window (w)) {
-	
+    
         GtkStyle *style = gtk_widget_get_style(w);
 
         /* fixme: */
@@ -197,29 +197,30 @@ void sp_event_context_update_cursor(SPEventContext *ec) {
                     SP_RGBA32_U_COMPOSE(SP_RGBA32_R_U(strokeColor),SP_RGBA32_G_U(strokeColor),SP_RGBA32_B_U(strokeColor),SP_COLOR_F_TO_U(strokeOpacity))
                     );
                 if (pixbuf != NULL) {
-                    if (ec->cursor)
+                    if (ec->cursor) {
 #if GTK_CHECK_VERSION(3,0,0)
                         g_object_unref(ec->cursor);
 #else
                         gdk_cursor_unref(ec->cursor);
 #endif
+                    }
                     ec->cursor = gdk_cursor_new_from_pixbuf(display, pixbuf, ec->hot_x, ec->hot_y);
                     g_object_unref(pixbuf);
                 }
             } else {
-		GdkPixbuf *pixbuf = gdk_pixbuf_new_from_xpm_data((const gchar **)ec->cursor_shape);
+        GdkPixbuf *pixbuf = gdk_pixbuf_new_from_xpm_data((const gchar **)ec->cursor_shape);
 
                 if (pixbuf) {
-                    if (ec->cursor)
+                    if (ec->cursor) {
 #if GTK_CHECK_VERSION(3,0,0)
                         g_object_unref(ec->cursor);
 #else
                         gdk_cursor_unref(ec->cursor);
 #endif
+                    }
                     ec->cursor = gdk_cursor_new_from_pixbuf(display,
-				    pixbuf, ec->hot_x, ec->hot_y);
-                
-		    g_object_unref(pixbuf);
+                    pixbuf, ec->hot_x, ec->hot_y);
+                    g_object_unref(pixbuf);
                 }
             }
         }
@@ -941,7 +942,7 @@ gint sp_event_context_root_handler(SPEventContext * event_context,
                 DelayedSnapEvent::EVENTCONTEXT_ROOT_HANDLER);
         break;
     case GDK_BUTTON_RELEASE:
-        if (event_context->_delayed_snap_event) {
+        if (event_context && event_context->_delayed_snap_event) {
             // If we have any pending snapping action, then invoke it now
             sp_event_context_snap_watchdog_callback(
                     event_context->_delayed_snap_event);
@@ -982,7 +983,7 @@ gint sp_event_context_item_handler(SPEventContext * event_context,
         sp_event_context_snap_delay_handler(event_context, (gpointer) item, NULL, (GdkEventMotion *) event, DelayedSnapEvent::EVENTCONTEXT_ITEM_HANDLER);
         break;
     case GDK_BUTTON_RELEASE:
-        if (event_context->_delayed_snap_event) {
+        if (event_context && event_context->_delayed_snap_event) {
             // If we have any pending snapping action, then invoke it now
             sp_event_context_snap_watchdog_callback(event_context->_delayed_snap_event);
         }
@@ -1039,8 +1040,8 @@ void sp_event_root_menu_popup(SPDesktop *desktop, SPItem *item, GdkEvent *event)
     if (event->type == GDK_KEY_PRESS) {
         item = sp_desktop_selection(desktop)->singleItem();
     }
-	ContextMenu* CM = new ContextMenu(desktop, item);
-	CM->show();
+    ContextMenu* CM = new ContextMenu(desktop, item);
+    CM->show();
 
     switch (event->type) {
     case GDK_BUTTON_PRESS:
@@ -1285,9 +1286,16 @@ gboolean sp_event_context_snap_watchdog_callback(gpointer data) {
     }
 
     SPEventContext *ec = dse->getEventContext();
-    if (ec == NULL || ec->desktop == NULL) {
+    if (ec == NULL) {
+        delete dse;
+        return false;
+    }
+    if (ec->desktop == NULL) {
+        ec->_delayed_snap_event = NULL;
+        delete dse;
         return false;
     }
+    
     ec->_dse_callback_in_process = true;
 
     SPDesktop *dt = ec->desktop;
@@ -1316,8 +1324,26 @@ gboolean sp_event_context_snap_watchdog_callback(gpointer data) {
         break;
     case DelayedSnapEvent::CONTROL_POINT_HANDLER: {
         using Inkscape::UI::ControlPoint;
-        ControlPoint *point = reinterpret_cast<ControlPoint*> (dse->getItem2());
-        point->_eventHandler(ec, dse->getEvent());
+        gpointer pitem2 = dse->getItem2();
+        if (!pitem2)
+        {
+            ec->_delayed_snap_event = NULL;
+            delete dse;
+            return false;
+        }
+        ControlPoint *point = reinterpret_cast<ControlPoint*> (pitem2);
+        if (point) {
+            if (point->position().isFinite() && (dt == point->_desktop)) {            
+                point->_eventHandler(ec, dse->getEvent());
+            }
+            else {
+                //workaround:
+                //[Bug 781893] Crash after moving a Bezier node after Knot path effect?
+                // --> at some time, some point with X = 0 and Y = nan (not a number) is created ...
+                //     even so, the desktop pointer is invalid and equal to 0xff
+                g_warning ("encountered non finite point when evaluating snapping callback");
+            }
+        }
     }
         break;
     case DelayedSnapEvent::GUIDE_HANDLER: {
diff --git a/src/knot.cpp b/src/knot.cpp
index ad8bca2b9..afcd70eb5 100644
--- a/src/knot.cpp
+++ b/src/knot.cpp
@@ -32,9 +32,9 @@
 using Inkscape::DocumentUndo;
 
 #define KNOT_EVENT_MASK (GDK_BUTTON_PRESS_MASK | GDK_BUTTON_RELEASE_MASK | \
-			 GDK_POINTER_MOTION_MASK | \
-			 GDK_POINTER_MOTION_HINT_MASK | \
-			 GDK_KEY_PRESS_MASK | GDK_KEY_RELEASE_MASK)
+             GDK_POINTER_MOTION_MASK | \
+             GDK_POINTER_MOTION_HINT_MASK | \
+             GDK_KEY_PRESS_MASK | GDK_KEY_RELEASE_MASK)
 
 static bool nograb = false;
 
@@ -75,13 +75,13 @@ GType sp_knot_get_type()
     if (!type) {
         GTypeInfo info = {
             sizeof(SPKnotClass),
-            NULL,	/* base_init */
-            NULL,	/* base_finalize */
+            NULL,    /* base_init */
+            NULL,    /* base_finalize */
             (GClassInitFunc) sp_knot_class_init,
-            NULL,	/* class_finalize */
-            NULL,	/* class_data */
+            NULL,    /* class_finalize */
+            NULL,    /* class_data */
             sizeof (SPKnot),
-            16,	/* n_preallocs */
+            16,    /* n_preallocs */
             (GInstanceInitFunc) sp_knot_init,
             NULL
         };
@@ -138,13 +138,13 @@ static void sp_knot_class_init(SPKnotClass *klass)
                                           G_TYPE_UINT);
 
     knot_signals[UNGRABBED] = g_signal_new("ungrabbed",
-					    G_TYPE_FROM_CLASS(klass),
-					    G_SIGNAL_RUN_FIRST,
-					    G_STRUCT_OFFSET(SPKnotClass, ungrabbed),
-					    NULL, NULL,
-					    g_cclosure_marshal_VOID__UINT,
-					    G_TYPE_NONE, 1,
-					    G_TYPE_UINT);
+                        G_TYPE_FROM_CLASS(klass),
+                        G_SIGNAL_RUN_FIRST,
+                        G_STRUCT_OFFSET(SPKnotClass, ungrabbed),
+                        NULL, NULL,
+                        g_cclosure_marshal_VOID__UINT,
+                        G_TYPE_NONE, 1,
+                        G_TYPE_UINT);
 
     knot_signals[MOVED] = g_signal_new("moved",
                                         G_TYPE_FROM_CLASS(klass),
@@ -323,14 +323,14 @@ static int sp_knot_handler(SPCanvasItem */*item*/, GdkEvent *event, SPKnot *knot
             }
             break;
     case GDK_BUTTON_PRESS:
-            if (event->button.button == 1 && !knot->desktop->event_context->space_panning) {
+            if ((event->button.button == 1) && knot->desktop && knot->desktop->event_context && !knot->desktop->event_context->space_panning) {
                 Geom::Point const p = knot->desktop->w2d(Geom::Point(event->button.x, event->button.y));
                 sp_knot_start_dragging(knot, p, (gint) event->button.x, (gint) event->button.y, event->button.time);
                 consumed = TRUE;
             }
             break;
     case GDK_BUTTON_RELEASE:
-            if (event->button.button == 1 && !knot->desktop->event_context->space_panning) {
+            if (event->button.button == 1 && knot->desktop && knot->desktop->event_context && !knot->desktop->event_context->space_panning) {
                 // If we have any pending snap event, then invoke it now
                 if (knot->desktop->event_context->_delayed_snap_event) {
                     sp_event_context_snap_watchdog_callback(knot->desktop->event_context->_delayed_snap_event);
@@ -368,7 +368,7 @@ static int sp_knot_handler(SPCanvasItem */*item*/, GdkEvent *event, SPKnot *knot
             }
             break;
     case GDK_MOTION_NOTIFY:
-            if (grabbed && !knot->desktop->event_context->space_panning) {
+            if (grabbed && knot->desktop && knot->desktop->event_context && !knot->desktop->event_context->space_panning) {
                 consumed = TRUE;
 
                 if ( within_tolerance
@@ -404,7 +404,7 @@ static int sp_knot_handler(SPCanvasItem */*item*/, GdkEvent *event, SPKnot *knot
             sp_knot_set_flag(knot, SP_KNOT_MOUSEOVER, TRUE);
             sp_knot_set_flag(knot, SP_KNOT_GRABBED, FALSE);
 
-            if (knot->tip) {
+            if (knot->tip && knot->desktop && knot->desktop->event_context) {
                 knot->desktop->event_context->defaultMessageContext()->set(Inkscape::NORMAL_MESSAGE, knot->tip);
             }
 
@@ -416,7 +416,7 @@ static int sp_knot_handler(SPCanvasItem */*item*/, GdkEvent *event, SPKnot *knot
             sp_knot_set_flag(knot, SP_KNOT_MOUSEOVER, FALSE);
             sp_knot_set_flag(knot, SP_KNOT_GRABBED, FALSE);
 
-            if (knot->tip) {
+            if (knot->tip && knot->desktop && knot->desktop->event_context) {
                 knot->desktop->event_context->defaultMessageContext()->clear();
             }
 
diff --git a/src/ui/tool/control-point.cpp b/src/ui/tool/control-point.cpp
index fbc9858cc..8c4924f26 100644
--- a/src/ui/tool/control-point.cpp
+++ b/src/ui/tool/control-point.cpp
@@ -211,6 +211,9 @@ void ControlPoint::_setPixbuf(Glib::RefPtr<Gdk::Pixbuf> p)
 // re-routes events into the virtual function
 int ControlPoint::_event_handler(SPCanvasItem */*item*/, GdkEvent *event, ControlPoint *point)
 {
+    if ((point == NULL) || (point->_desktop == NULL)) {
+        return FALSE;
+    }
     return point->_eventHandler(point->_desktop->event_context, event) ? TRUE : FALSE;
 }
 
@@ -220,6 +223,24 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
     // NOTE the static variables below are shared for all points!
     // TODO handle clicks and drags from other buttons too
 
+    if (event == NULL)
+    {
+        return false;
+    }
+    
+    if (event_context == NULL)
+    {
+        return false;
+    }
+    if (_desktop == NULL)
+    {
+        return false;
+    }
+    if(event_context->desktop !=_desktop)
+    {
+        g_warning ("ControlPoint: desktop pointers not equal!");
+        //return false;
+    }
     // offset from the pointer hotspot to the center of the grabbed knot in desktop coords
     static Geom::Point pointer_offset;
     // number of last doubleclicked button
@@ -227,7 +248,8 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
     
     Inkscape::Preferences *prefs = Inkscape::Preferences::get();
     int drag_tolerance = prefs->getIntLimited("/options/dragtolerance/value", 0, 0, 100);
-    
+    GdkEventMotion em;
+    SPCanvas* Ca;
     switch(event->type)
     {   
     case GDK_BUTTON_PRESS:
@@ -253,18 +275,22 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
         return true;
         
     case GDK_MOTION_NOTIFY:
-        combine_motion_events(_desktop->canvas, event->motion, 0);
-        if (_event_grab && !_desktop->event_context->space_panning) {
+        Ca = _desktop->canvas;
+        em = event->motion;
+        combine_motion_events(Ca, em, 0);
+        if (_event_grab && ! event_context->space_panning) {
             _desktop->snapindicator->remove_snaptarget(); 
             bool transferred = false;
             if (!_drag_initiated) {
-                bool t = fabs(event->motion.x - _drag_event_origin[Geom::X]) <= drag_tolerance &&
-                         fabs(event->motion.y - _drag_event_origin[Geom::Y]) <= drag_tolerance;
-                if (t) return true;
+                bool t = fabs(em.x - _drag_event_origin[Geom::X]) <= drag_tolerance &&
+                         fabs(em.y - _drag_event_origin[Geom::Y]) <= drag_tolerance;
+                if (t){
+                    return true;
+                }
 
                 // if we are here, it means the tolerance was just exceeded.
                 _drag_origin = _position;
-                transferred = grabbed(&event->motion);
+                transferred = grabbed(&em);
                 // _drag_initiated might change during the above virtual call
                 if (!_drag_initiated) {
                     // this guarantees smooth redraws while dragging
@@ -275,15 +301,14 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
             if (!transferred) {
                 // dragging in progress
                 Geom::Point new_pos = _desktop->w2d(event_point(event->motion)) + pointer_offset;
-                
                 // the new position is passed by reference and can be changed in the handlers.
-                dragged(new_pos, &event->motion);
+                dragged(new_pos, &em);
                 move(new_pos);
-                _updateDragTip(&event->motion); // update dragging tip after moving to new position
+                _updateDragTip(&em); // update dragging tip after moving to new position
                 
                 _desktop->scroll_to_point(new_pos);
                 _desktop->set_coordinate_status(_position);
-                sp_event_context_snap_delay_handler(_desktop->event_context, NULL,
+                sp_event_context_snap_delay_handler(event_context, NULL,
                     (gpointer) this, &event->motion,
                     DelayedSnapEvent::CONTROL_POINT_HANDLER);
             }
@@ -299,8 +324,9 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
             // We must snap at some point in time though, and this is our last chance)
             // PS: For other contexts this is handled already in sp_event_context_item_handler or
             // sp_event_context_root_handler
-            if (_desktop->event_context->_delayed_snap_event) {
-                sp_event_context_snap_watchdog_callback(_desktop->event_context->_delayed_snap_event);
+            //if (_desktop && _desktop->event_context && _desktop->event_context->_delayed_snap_event) {
+            if (event_context->_delayed_snap_event) {
+                sp_event_context_snap_watchdog_callback(event_context->_delayed_snap_event);
             }
 
             sp_canvas_item_ungrab(_canvas_item, event->button.time);
@@ -335,8 +361,9 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
         if (_event_grab && !event->grab_broken.keyboard) {
             {
                 ungrabbed(NULL);
-                if (_drag_initiated)
+                if (_drag_initiated) {
                     _desktop->canvas->endForcedFullRedraws();
+                }
             }
             _setState(STATE_NORMAL);
             _event_grab = false;
@@ -355,7 +382,7 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
             if (!_drag_initiated) break;
 
             // temporarily disable snapping - we might snap to a different place than we were initially
-            sp_event_context_discard_delayed_snap_event(_desktop->event_context);
+            sp_event_context_discard_delayed_snap_event(event_context);
             SnapPreferences &snapprefs = _desktop->namedview->snap_manager.snapprefs;
             bool snap_save = snapprefs.getSnapEnabledGlobally();
             snapprefs.setSnapEnabledGlobally(false);
@@ -389,7 +416,8 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
 
             ungrabbed(NULL); // ungrabbed handlers can handle a NULL event
             snapprefs.setSnapEnabledGlobally(snap_save);
-            } return true;
+            }
+            return true;
         case GDK_KEY_Tab:
             {// Downcast from ControlPoint to TransformHandle, if possible
              // This is an ugly hack; we should have the transform handle intercept the keystrokes itself
@@ -415,7 +443,9 @@ bool ControlPoint::_eventHandler(SPEventContext *event_context, GdkEvent *event)
         }
         // Do not break here, to allow for updating tooltips and such
     case GDK_KEY_RELEASE: 
-        if (mouseovered_point != this) return false;
+        if (mouseovered_point != this){
+            return false;
+        }
         if (_drag_initiated) {
             return true; // this prevents the tool from overwriting the drag tip
         } else {
diff --git a/src/ui/tool/control-point.h b/src/ui/tool/control-point.h
index 835fa5ab3..30efe8a27 100644
--- a/src/ui/tool/control-point.h
+++ b/src/ui/tool/control-point.h
@@ -182,6 +182,7 @@ public:
 
     // temporarily public, until snap delay is refactored a little
     virtual bool _eventHandler(SPEventContext *event_context, GdkEvent *event);
+    SPDesktop *const _desktop; ///< The desktop this control point resides on.
 
 protected:
 
@@ -335,7 +336,6 @@ protected:
 
     virtual bool _hasDragTips() const { return false; }
 
-    SPDesktop *const _desktop; ///< The desktop this control point resides on.
 
     SPCanvasItem * _canvas_item; ///< Visual representation of the control point.
 
diff --git a/src/ui/tool/event-utils.cpp b/src/ui/tool/event-utils.cpp
index c3b69d79d..079275d63 100644
--- a/src/ui/tool/event-utils.cpp
+++ b/src/ui/tool/event-utils.cpp
@@ -55,6 +55,9 @@ unsigned combine_key_events(guint keyval, gint mask)
 
 unsigned combine_motion_events(SPCanvas *canvas, GdkEventMotion &event, gint mask)
 {
+    if (canvas == NULL) {
+        return false;
+    }
     GdkEvent *event_next;
     gint i = 0;
     event.x -= canvas->x0;
@@ -62,7 +65,7 @@ unsigned combine_motion_events(SPCanvas *canvas, GdkEventMotion &event, gint mas
 
     event_next = gdk_event_get();
     // while the next event is also a motion notify
-    while (event_next && event_next->type == GDK_MOTION_NOTIFY
+    while (event_next && (event_next->type == GDK_MOTION_NOTIFY)
             && (!mask || event_next->motion.state & mask))
     {
         if (event_next->motion.device == event.device) {
@@ -86,8 +89,9 @@ unsigned combine_motion_events(SPCanvas *canvas, GdkEventMotion &event, gint mas
         i++;
     }
     // otherwise, put it back onto the queue
-    if (event_next)
+    if (event_next) {
         gdk_event_put(event_next);
+    }
     event.x += canvas->x0;
     event.y += canvas->y0;
 
diff --git a/src/ui/tool/node.h b/src/ui/tool/node.h
index b3964a257..591dd8532 100644
--- a/src/ui/tool/node.h
+++ b/src/ui/tool/node.h
@@ -309,14 +309,14 @@ public:
     // default copy, default assign
 
     self &operator++() {
-        _node = _node->ln_next;
+        _node = (_node?_node->ln_next:NULL);
         return *this;
     }
     self &operator--() {
-        _node = _node->ln_prev;
+        _node = (_node?_node->ln_prev:NULL);
         return *this;
     }
-    bool operator==(self const &other) const { return _node == other._node; }
+    bool operator==(self const &other) const { if(&other){return _node == other._node;} else{return false;} }
     N &operator*() const { return *static_cast<N*>(_node); }
     inline operator bool() const; // define after NodeList
     /// Get a pointer to the underlying node. Equivalent to <code>&*i</code>.