Re: segfault when creating new objects

[Hans Breuer <hans breuer org>]

(somehow this answer stayed in my Drafts folder)

At 20.12.2008 09:07, Sameer Sahasrabuddhe wrote:
> New patch attached.
>
> On Sat, Dec 20, 2008 at 3:13 AM, Hans Breuer <hans breuer org> 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 
That change looks wrong. g_return_if_fail is meant to be validating the API
contract. It is just not usuful to ask for creation of an object but not
giving the type. Replacing all the g_return_if_fail() just decreases
readibility in my opinion.

> 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.
I tend to disagree, this depends on the developer. I've seen a lot of
patches asked for inclusion which produce warnings.

But more important: these macros are too new to be used unconditional. Dia
still requires only glib/gtk 2.6 but g_warn_if_* got added with glib-2-16.

> > Also your patch would partially revert revision 3876 (for the above
> > mentioned case):
> >
> > 2008-01-10  Hans Breuer  <hans breuer org>
> >
> >  * 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). 
So bzr blame is wrong (or at least not giving the correct SVN revisions):

Revision: 3716
Author: sdteffen
Date: 22:30:52, Dienstag, 10. Juli 2007
Message:
ALL_LINGUAS are in one line again
----
Modified : /trunk/ChangeLog
Modified : /trunk/configure.in


But:

Revision: 3876
Author: hans
Date: 23:16:02, Donnerstag, 10. Januar 2008
Message:
2008-01-10  Hans Breuer  <hans breuer org>

  * app/create_object.c(create_object_button_release) : don't crash on
  tool->obj being NULL, just do nothing than.

  [...]

----
Modified : /trunk/ChangeLog
Modified : /trunk/app/create_object.c
  [...]


Not sure which revisions got lost during conversion, hope they were not 
importenat ;)

> That's exactly the
> crash that I saw, but in a different place, as describe below.
should be fixed now, see respective bug report.


Thanks,
        Hans

-------- Hans "at" Breuer "dot" Org -----------
Tell me what you need, and I'll tell you how to
get along without it.                -- Dilbert