New patch attached.
On Sat, Dec 20, 2008 at 3:13 AM, Hans Breuer <[email protected]> wrote:
> Sorry, I don't see how adding g_return_if_fail() should ensure anything.
Ok. My mistake. What I want is a warning followed by a return. Updated
the patch accordingly. The new patch also fixes an existing use of
g_return_if_fail() in lib/object_defaults.c Note the use of
g_warn_if_reached() instead of g_warning(), since the former provides
the location in the code ... an actual warning text is not important
since the developer will examine the code anyway.
> Also your patch would partially revert revision 3876 (for the above
> mentioned case):
>
> 2008-01-10 Hans Breuer <[email protected]>
>
> * app/create_object.c(create_object_button_release) : don't crash on
> tool->obj being NULL, just do nothing than.
I think you meant revision 3716 (from bzr blame). That's exactly the
crash that I saw, but in a different place, as describe below.
>> I actually encountered this because of another
>> problem with the DATADIR setting. I suppose the CreateObjectTool
>> rarely fails in normal environments.
>>
> It rarely does but still should not crash.
It does crash in create_object_button_press(), in app/create_object.c
... the call to dia_object_default_create() can possibly return a
NULL, which is not checked. The pointer check intended in 3716 checks
for this is in _button_release(), but it should also be checked in
_button_press() and _motion(). It is sufficient to place the related
warning only in _button_press().
Sameer.
--
http://www.it.iitb.ac.in/~sameerds/
=== modified file 'app/create_object.c'
--- app/create_object.c 2008-04-19 14:12:06 +0000
+++ app/create_object.c 2008-12-20 07:53:58 +0000
@@ -46,7 +46,6 @@
Handle *handle1;
Handle *handle2;
DiaObject *obj;
- real click_distance;
ddisplay_untransform_coords(ddisp,
(int)event->x, (int)event->y,
@@ -56,11 +55,15 @@
snap_to_grid(ddisp, &clickedpoint.x, &clickedpoint.y);
- click_distance = ddisplay_untransform_length(ddisp, 3.0);
-
obj = dia_object_default_create (tool->objtype, &clickedpoint,
tool->user_data,
&handle1, &handle2);
+ tool->obj = obj; /* ensure that tool->obj is initialised in case we
+ return early. */
+ if (!obj) {
+ g_warn_if_reached();
+ return;
+ }
diagram_add_object(ddisp->diagram, obj);
@@ -81,8 +84,6 @@
}
diagram_select(ddisp->diagram, obj);
- tool->obj = obj;
-
/* Connect first handle if possible: */
if ((handle1!= NULL) &&
(handle1->connect_type != HANDLE_NONCONNECTABLE)) {
@@ -125,9 +126,8 @@
GList *parent_candidates;
- g_return_if_fail (obj != NULL);
- if (!obj) /* not sure if this isn't enough */
- return; /* could be a legal invariant */
+ if (!obj)
+ return;
if (tool->moving) {
gdk_pointer_ungrab (event->time);
@@ -212,6 +212,9 @@
if (!tool->moving)
return;
+ if (!tool->obj)
+ return;
+
ddisplay_untransform_coords(ddisp, event->x, event->y, &to.x, &to.y);
/* make sure the new object is restricted to its parent */
=== modified file 'lib/object_defaults.c'
--- lib/object_defaults.c 2008-05-25 12:59:25 +0000
+++ lib/object_defaults.c 2008-12-20 07:41:52 +0000
@@ -266,28 +266,31 @@
const DiaObject *def_obj;
DiaObject *obj;
- g_return_val_if_fail (type != NULL, NULL);
+ if (!type) {
+ g_warn_if_reached();
+ return NULL;
+ }
+
+ obj = type->ops->create (startpoint, user_data, handle1, handle2);
- /* don't use dia_object_default_get() as it would insert the object into the hashtable (store defaults without being asked for it) */
+ if (!obj) {
+ g_warn_if_reached();
+ return NULL;
+ }
+
+ /* don't use dia_object_default_get() as it would insert the object
+ into the hashtable (store defaults without being asked for it) */
def_obj = g_hash_table_lookup (defaults_hash, type->name);
- if (def_obj && def_obj->ops->describe_props)
- {
- /* copy properties to new object, but keep position */
- obj = type->ops->create (startpoint, user_data, handle1, handle2);
- if (obj)
- {
- GPtrArray *props = prop_list_from_descs (
- object_get_prop_descriptions(def_obj), pdtpp_standard_or_defaults);
- def_obj->ops->get_props(def_obj, props);
- obj->ops->set_props(obj, props);
- obj->ops->move (obj, startpoint);
- prop_list_free(props);
- }
- }
- else
- {
- obj = type->ops->create (startpoint, user_data, handle1, handle2);
- }
+
+ if (def_obj && def_obj->ops->describe_props) {
+ /* copy properties to new object, but keep position */
+ GPtrArray *props = prop_list_from_descs(object_get_prop_descriptions(def_obj),
+ pdtpp_standard_or_defaults);
+ def_obj->ops->get_props(def_obj, props);
+ obj->ops->set_props(obj, props);
+ obj->ops->move (obj, startpoint);
+ prop_list_free(props);
+ }
return obj;
}
_______________________________________________
dia-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/dia-list
FAQ at http://live.gnome.org/Dia/Faq
Main page at http://live.gnome.org/Dia
