Re: Submenu breakage

[Tim Janik <timj gtk org>]

On 17 Feb 2001, Owen Taylor wrote:

> > On 7 Feb 2001, Owen Taylor wrote:

> > > > Has anyone actually looked deeper in the submenu horkage? Is it 100% sure
> > > > that it is the boxed type changes that causes it. 
> > > 
> > > Well, the boxed typed stuff BREAKS a lot of stuff BADLY, so it's probably 
> > > not worth debugging until we get the boxed type fixed.
> > 
> > that's exagerating, boxed copying breaks signals that pass boxed
> > arguments in to signal handlers and rely on them to make modifications
> > which have to be available to the caller after emission.
> 
> Which of course, is only rare, unusual signals like ::size_request...

very funny.

> > that in itself should not break menus as menus don't have strange
> > text iterators or non-NULL terminated strings.
> 
> Or GtkRequisition, or GtkSelectionData, or...

blargh, you could claim to have a point if those were actually announced
to the signal system as boxed types, but currently the code still uses
G_TYPE_POINTER. my point was that this could _not_ break the submenu code.

> > however the boxed type copying uses gdk_event_copy() upon GtkWidget::event
> > now, and that relies on the event structures properly setup.
> > now gtk_menu_motion_notify() creates a syntesized event that's
> > only half setup:
> > 
> >           GdkEvent send_event;
> > 
> >           send_event.crossing.type = GDK_ENTER_NOTIFY;
> >           send_event.crossing.window = event->window;
> >           send_event.crossing.time = event->time;
> >           send_event.crossing.send_event = TRUE;
> >           send_event.crossing.x_root = event->x_root;
> >           send_event.crossing.y_root = event->y_root;
> >           send_event.crossing.x = event->x;
> >           send_event.crossing.y = event->y;
> > 
> > specifically send_event.crossing.subwindow can contain garbage this way,
> > which will obviously break gdk_window_ref (event->crossing.subwindow) in 
> > gdk_event_copy().
> > 
> > i just had to figure that plain:
> > 
> >           GdkEvent send_event = { 0, };
> > 
> > won't fix this because default-zero-initialization of unions only
> > applies to the first member, so that code actually needs to read:
> > 
> >           GdkEvent send_event;
> >           memset (&send_event, 0, sizeof (send_event));
> 
> How about 
> 
>  GdkEventCrossing send_event = { 0 };
> 
>  [...]
> 
>  gtk_widget_event (widget, (GdkEvent *)send_event);

that would have worked, but it's still important to point out
that GdkEvent ev = { 0, }; doesn't so that trap is avoided in the future.

> Seems just a bit cleaner to me. (Or, simply filling in the
> missing subwindow field explicitely....)

filling in just the subwindow field is a bad idea since that leaves
garbage in the rest of the fields that aren't explicitely setup, e.g.
GdkCrossingMode mode;
just because leaving the crossing mode filled with garbage doesn't trigger
an immediate segfault doesn't mean the code is be correct.
and we have had quite a bunch of cases where synthesized events weren't
completely setup and probably still have. so the baseline is that just
relying on explicit field setup isn't good enough. people don't have
enough discipline to actually lookup all the fields that need initialization
and its also a maintenance nightmare when extending GdkEvent* structures.

> 
>                                         Owen
> 

---
ciaoTJ