Re: [jlbec evilplan org: Enumeration patch]

[Alexander Larsson <alla lysator liu se>]

On Thu, 19 Oct 2000, Joel Becker wrote:

> On Thu, Oct 19, 2000 at 10:38:44AM +0200, Alexander Larsson wrote:
> > I like this idea. It doesn't solve all problems concerning list-returning
> > behaviour though. It is still necessary in some cases to know if the
> > enumerator is a static copy of the container state when it was enumerated,
> > or if it is using the live state and is invalidated whenever the container
> > is modified. 
> 
> 	I would lean towards making it always invalidated.  I don't like
> the caller knowing static/live state (part of the point of the
> enumeration), and the caller should be able to know when the state
> changes.  Anyone reading something should know to lock against writes,
> etc.  The usual stuff.  So in a single-threaded app, if the user makes a
> modifying call, the enumeration is invalidated.  If a multi-threaded
> app, the caller should do the usual shared-read/single-write locking
> around a piece of data.

Ok, this seems like nice behaviour.
 
> > If the GEnumerationFunc got a pointer to the gpointer context the list
> > enumerator (and other simple enumerators) won't have to allocate a special
> > context struct (a list-item in the list-enumerator case). I think that
> > would make the code simpler.
> > 
> > typedef gpointer (*GEnumerationFunc)	(gpointer *context);
> 
> 	Yes, I thought about that.  Part of me really doesn't want even
> the enumeration creator mucking with the actual GEnumeration structure.
> I was sort of thinking of the context pointer as opaque.  You can't
> change it after creation.  So while it adds that extra step to the
> simple list enumerator, it protects against misuse in other situations.
> Most folks would probably use _new_from_list() and never see that.
> Anyone requiring a more complex enumerator will probably pass in their
> own structure, which will never change address.  eg:
> 
> GtkContainer *foo;
> 
> g_enumeration_new(foo, has_more, get_next, destroy_notify);
> 
> has_more(gpointer context)
> {
>     GtkWidget *foo = GTK_WIDGET(context);
> 
>     /* access only parts of the foo structure here */
> 
>     return retval;
> }

No, that is not good enough. You cannot have a pointer to the GtkContainer
only. You must have a pointer to the current element of the enumeration,
which you can modify on each get_next. Something like:

struct FooEnumerationContext {
   int n_elements;
   element *current;  
}
GtkFoo *foo;
context = g_new(FooEnumerationContext, 1);
context.n_elements = foo->n_elements;
context.current = foo->element[0];
g_enumeration_new(context, has_more, get_next, destroy_notify);

In this particular case you need the foo object reference to see when
you've passed the end of the array, but often (as in the case of
lists, which are probably the most used enumerator) you don't need that,
but only a single pointer. Then you could get away with less allocations
and dereferences.
 
> > If, in the list-enumerator, you return the current element and then
> > increment the enumerator you wouldn't have to prepend an empty element to
> > the list.
> 
> 	Right, but then at the end of the list I've got a NULL pointer
> and no reference back to the list to g_list_free() it upon
> destroy_notify.

This is easily fixed by freeing the enumerator list elements while
enumerating. That is a good optimization anyway.

/ Alex