On Fri, Jun 03, 2005 at 09:14:19AM -0400, Derek Atkins wrote: > Quoting Chris Shoemaker <[EMAIL PROTECTED]>: > <snip> > > I really don't think there's a bug here... There was a use-after-free but it > was from the context, not the widget. Hampton fixed that. I really don't > think we need to re-architect the rest of the code. It works just fine, and > all the rest of gnucash uses this same model: > > CM close callback tells the widget to destroy itself > Gtk Destroy callback frees the context and clears the references. > > So I don't think we need to do anything. What we've got works just fine > (modulo > the bug that hampton fixed already).
It's better to design out a bug than to patch it out. The existing pattern is bad for many reasons. 1) It's an abuse of the documented gtk destroy api, which is only suppose to break references, NOT free anything. (Freeing is handled by finalize, and only when refcount = 0.) 2) High coupling: currently, the destroy handler takes the cw as user-data and can pillage all the fields of the cw struct before finally freeing something that doesn't even depend on the dialog. In the correct usage of destroy, it should only receive the component_id through userdata -- just enough to break external references, no more, no less -- low coupling. 3) Low cohesion: currently, the destroy handler for the dialog has to know tons about the cw struct and about the CM, including CM unregistration. Likewise, the close handler has to know about all the side-effects of the destroy handler. In a high cohesion design, the destroy handler only has to know how to break the single external reference, and the close callback rightly accesses the data fields for the component that generated the event. The close handler doesn't have to know any more about the destroy signal than what I can read in the GTK docs: it breaks refs. BTW, I'm _really_ not a design nazi. I just _look_ like one next to you. :) You seem to think that anything in GC that works must be a good design. You might try being a bit more open-minded about proposals for design improvements, especially coming from people who've proven a willingness to code, and especially when those proposals don't require any action on your part. If you think a proposal is really bad, try to make _constructive_ recommendations for improvement, (GC can always be improved, you know) and not be so disparaging. I'm seeing that this "we've always done it that way; we do that everywhere; it works; we don't need to change anything... etc." attitude is a disturbing and recurring pattern. Maybe being more honest about GC's design, ehm, weaknesses, would encourage more developer participation. Just a thought. -chris _______________________________________________ gnucash-devel mailing list [email protected] https://lists.gnucash.org/mailman/listinfo/gnucash-devel
