Re: [PATCH] ANSI C correctness + warning fixes

[otaylor redhat com]

Martin Baulig <martin home-of-linux org> writes:

> otaylor fresnel labs redhat com writes:
> 
> > ===========
> > +  { GDK_ACTION_ASK,   (const char *) action_ask_bits,  (const char *) action_ask_mask_bits,  NULL },
> > +  { GDK_ACTION_COPY,  (const char *) action_copy_bits, (const char *) action_copy_mask_bits, NULL },
> > +  { GDK_ACTION_MOVE,  (const char *) action_move_bits, (const char *) action_move_mask_bits, NULL },
> > +  { GDK_ACTION_LINK,  (const char *) action_link_bits, (const char *) action_link_mask_bits, NULL },
> > +  { 0              ,  (const char *) action_none_bits, (const char *) action_none_mask_bits, NULL },
> > ============
> > 
> > Please change the type of these pointers to guchar * and move the
> > casts to when gdk_bitmap_create_from_data() is called. (Since I'd
> > consider the prototype of that function the problem.)
 
> Hmm, what about changing the prototype of that function to take
> const guchar * arguments ?

Possible; though it is a backwards incompatible change, and I'm
reluctant to make it for that reason.

But actually, I think the xbm format is defined to have char *
not unsigned char * in the structure definitions; I'm not
sure why the inline xbm's in gtkdnd.c have guchar *. Probably
the best thing to do is to simply change them and the structure
fields to gchar *.
 
> > ===========
> > -  klass->add_accelerator = (void*) gtk_accel_group_handle_add;
> > -  klass->remove_accelerator = (void*) gtk_accel_group_handle_remove;
> > +  klass->add_accelerator = gtk_widget_real_add_accelerator;
> > +  klass->remove_accelerator = gtk_widget_real_remove_accelerator;
> >    klass->grab_focus = gtk_widget_real_grab_focus;
> >    klass->event = NULL;
> >    klass->button_press_event = NULL;
> > @@ -4907,4 +4918,30 @@ gtk_widget_class_path (GtkWidget *widget
> >        *path_p = g_strdup (rev_path);
> >        g_strreverse (*path_p);
> >      }
> > +}
> > +
> > +static gint
> > +gtk_widget_real_add_accelerator (GtkWidget *widget, guint accel_signal_id,
> > +				 GtkAccelGroup *accel_group, guint accel_key,
> > +				 GdkModifierType accel_mods, GtkAccelFlags accel_flags)
> > ===========
> > 
> > I'll let Tim comment on this one, but the thing is,
> > gtk_accel_group_handle_add() was meant to be used as the default
> > handler for remove_accelerator, so having to do a workaround like this
> > really cannot be the right way to do things.
> 
> Btw. I did this because gtk_accel_group_handle_add/remove() take GtkObject's
> but klass->add/remote_accelerator expects the first argument to be a
> GtkWidget, so we either need to cast the functions or introduce this static
> helper function.

I understand this. However, my comment is that handle_add() is meant
almost solely as a default handler for this virtual function slot,
so something needs to be fixed here.

Regards,
                                        Owen