Re: [gtk-list] Re: A couple of suggestions



On Wed, 8 Apr 1998, Lauri Alanko wrote:

> > > Or for the case when you want to use ints the "right way", having
> > > pointers to ints. Then you need to allocate an int for each of your
> > > keys. Then how are you supposed to remove it? All that's needed is a
> > > way to get the actual key object in the hash.
> > 
> > OK, I see your point now. Currently, you'd have to store a pointer
> > to the key in your value, if you wanted to be able to remove
> > single entries from the hash table.
> > 
> > Something like:
> > 
> > gboolean    g_hash_table_lookup_entry (GHashTable     *hash_table,
> >                                        const gpointer  key,
> >                                        const gpointer *key_return,
> >                                        const gpointer *val_return);
> > 
> > would probably be useful in such circumstances.
> 
> Yeps. I will add it. I'll do a bit of code cleanup, too, changing if
> (hash_table) {...} to g_return_if_fail (hash_table) which at least issues a
> warning.
> 
> I'll also add a function to force rehashing, if the user consciously does
> something that screws up the hash, such as modifying a key so that its hash
> value is different.
> 
> > > You never know of the future. Isn't it conceivable that some day, for
> > > efficiency reasons, you don't get an id but a direct pointer to a
> > > signal structure?
> > 
> > I suppose it is conceivable.
> 
> What is the idea with the signal ids, in any case? Pointers to unknown type
> are quite as opaque, and faster. And the extra level of indirection is
> confusing.

hm, i'd say this is a basic design decision. using signal ids (i mean the ids
that are assigned to newly introduced signals by the object classes) combined
with a fast hash lookup (the id can be used as a direct hash value, so the
lookup gets narrowed down to a simple slist search) gives the library the
possibility to check whether the given id (or opaque identifier) is valid or
not. if you would want to do this with opaque pointers you would still need a
hash lookup, so it doesn't really make a difference whether you pass an opaque
pointer or an id, though one might argue a guint id looks cleaner and doesn't
introduce problems for platforms with sizeof(void*)!=sizeof(unsigned int).

if you originally meant the handler ids (which are returned by the various
gtk_signal_connect.* functions), we are better of assigning each one a new
id rather then hope that the user will not pass in a pointer to a previously
used GtkHandler structure, which in fact will be realoocted quite often
during normal gtk usage. this would mess up the signal handling code in
major ways, i already had a hard time getting the handler structure handling
right just internaly ;)

> > > Oh, one more idea: a list that keeps track of both its
> > > ends. Basically:
> > > 
> > > struct _GDList{
> > > 	GList *head;
> > > 	GList *tail;
> > > };
> > > 
> > > And a similar one for GSLists. What's the use? Both access to head and 
> > > tail become O(1) operations, and you could use these without the
> > > bothersome list=g_list_foo(list) syntax.

it shouldn't be too difficult to have GSList*my_fifo_tail; besides
GSList*my_fifo; in cases where you really need a fifo in your code.
what about providing g_slist_append_with_tail (GSList *list_tail, gpointer
data) which will return the last (newly created) list pointer?

> > I've occasionally wanted something like that. The only real downside
> > would be a fair amount of code duplication in GLIB. And you 
> > could in most cases simply wrap the g_list functions.

adding a few more convenience functions for list operations seems more
appropriate for me, and it allowes to fit your needs without adding more
overhead.

> Not any more duplication than already with GList/GSList, I think. And the
> speed and syntax benefits weigh more, IMHO.
> 
> > (I don't see that much of a point in the GDSList, though - having the
> > tail isn't much use if you have to traverse the whole list to 
> > get the element before it)
> 
> Well, a GDSList would be optimal for a FIFO. You read from the head and add
> to the tail.
> 
> > > Oh, one more thing. Why not have 
> > > 
> > > typedef void (*GtkSignalFunc) ();
> > 
> > Sounds like a good suggestion to me. Though note that a large
> > fraction of GTK signals (all the _event ones, plus some others)
> > _do_ have return values. So the GTK_SIGNAL_FUNC() casts would
> > still be needed in many places.
> 
> All right, I'll change this too, if no one else has. Shouldn't this be moved
> to gtksignal.h, btw?

guint   gtk_object_class_add_user_signal (GtkObjectClass     *klass,
                                          const gchar        *name,
                                          GtkSignalMarshaller marshaller,
                                          GtkType             return_val,
                                          guint               nparams,
                                          ...);

is implemented in gtkobject.c for obvious reasons, and it depends on
GtkSignalMarshaller which in turn depends on GtkSignalFunc and the definition
for GTK_SIGNAL_FUNC() should be right besides GtkSignalFunc that's why
those are in gtkobject.h.

> 
> IMHO, handlers returning values is just syntactic sugar, though.. not sure if
> it's worth the marshalling overhead..

i think it is cleaner to use
gint
my_delete_block (GtkWidget *widget,
		 GdkEvent  *event)
{
  return TRUE;
}

than

void
my_delete_block (GtkWidget *widget,
		 GdkEvent  *event,
		 gint      *return_val)
{
  *return_val = TRUE;
}

and wouldn't even call this syntactic sugar, because the latter one requires
an extra pointer reference while the former passes the return value in
a register with most compiler implementations.

> Speaking of which, wouldn't it be sensible to have more marshallers than
> just the default one in gtksignal.h? At the very least a marshaller for
> signals that have one gpointer argument, these are needed everywhere.

there are actually not as much as one would think, a quick glance (using grep)
showes the following signals:

GtkWidget::draw
GtkWidget::size_request
GtkWidget::size_allocate
GtkWidget::selection_received
GtkWindow::set_focus

out of which the GtkWidget signals all use the same marshaller. if we do this,
people can start argue why do we favour gtk_default_marshaller_pointer over
gtk_default_marshaller_int or gtk_default_marshaller_uint. this is more a matter
of where to draw the line than anything else.

> 
> > It doesn't help things for people using C++, where the empty
> > parens are equivalent to (void) but it doesn't make things worse
> > for them either.
> 
> Ah, and regarding this, why are there extern "C" {..} :s wrapped around all
> headers? GTK has no support for C++, it's plain C code, so IMHO it's should
> be the C++ user's responsibility to say extern "C". Also, it screws up
> emacs' indenting. :)

i use this in my own programs:

#ifdef __cplusplus
extern "C" {
#pragma }
#endif /* __cplusplus */

it cures emacs indentation and i've never seen a compiler complain about it.
in fact they can't since #pragma is *meant* to be compiler specific and
therefore compilers who don't know about a specific pragma command are supposed
to ignore this silently.

> (Gee, why do I always forget something out?..)
> 
> _Yet_ one more thing, relating to the previous.
> 
> There are lots of
> 
> struct _Foo {int dummy;};
> 
>:s in glib.h and gdk.h. I can't see any justification for these. They make
> it look like the user code knows these types, and happily allows the user to
> allocate these.
> 
> The right thing to do would of course be just to have the
> 
> typedef struct _Foo Foo;
> 
> which has a forward declaration for struct _Foo, and allows the user to
> handle pointers to it, but not access it. Then the implementation code would
> have the definition for struct _Foo.
> 
> I can't think of any other reason for this than that Peter just overlooked
> this... Removing the dummy definitions shouldn't break anything, nor should
> then simplifying the implementations by removing the "real" type casting..

well, i think we should be pretty save with using only the forward
declarations.

> 
> 
> Lauri Alanko
> 

---
ciaoTJ



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]