Re: 04: glib: g_list_free_1 should return ->next

[Guido Draheim <guidod-2003- gmx de>]

Owen Taylor wrote:
> If you want to make feature enhancment requests, the 
> best procedure is:
>
>  - File them in bugzilla
>  - (optional) send mail to gtk-devel-list if you think
>    that discussion on the issue would be good.

the gtk.org docs say that feature requests shall be
sent to gtk-list gnome org, and so I did. The items
that I am going post are actually the result of a
code review and experiences with glib, I am not sure
if there is a need to file them just to have them
seen closed immediatly.

> I don't think this change makes sense though:
>
>  A) g_list_free_1() doesn't unlink the list node either,
>     so just returning next doesn't do any good. 
>     g_list_free_1() simply frees a single list node
>     that is already unlinked.
>  B) g_list_delete_link is exactly what you are looking for.

in other words,
list = g_list_delete_link (list, list);

looking through the fifteen places where g_list_free_1 is
currently used, that would be also true about there - so that
the free_1 would have to be depracated. It is not done since
it is known about there that we are the head and no unlink
operation is actually needed since it is intended to just
walk-and-free the list. It is actually a common task to do,
to have a "select" routine to return an allocated glist and
have the receiver walk-and-free the result-set.

The 04 notification shall simply tell you that there is room
for a better way of supporting this common operation. Otherwise,
just delete free_1 and see what your developers say.

cheers, guido

> Regards,
>                                         Owen
>
>
> Guido Draheim <guidod-2003- gmx de> writes:
>
>
> > The glib2 function g_list_free_1 is currently defined as
> > void     g_list_free_1         (GList            *list);
> >
> > This is not the best option, the function is actually
> > called in many places to free the head of a glist. In
> > about *all* places that includes an iteration on to the
> > ->next list item.
> >
> > Currently, all these places make up their own tmp variable
> > to store the ->next pointer for the iteration, ie. they
> > do something like
> >    GList* tmp = list->next;
> >    g_list_free_1 (list);
> >    list = tmp;
> >
> > All these code points may be collapsed if the g_list_free_1
> > function would be redeclared to return the ->next pointer
> > all by itself, with the code parts becoming simply:
> >    list = g_list_free_1 (list);
> >
> > Looking at the implementation of g_list_free_1, a developer
> > can easily see that the ->next value is accessed anyway, so
> > it would not be problematic to load the ->next value into
> > the return-register.
> >
> > GList*     g_list_free_1         (GList            *list);
> >
> > In effect, the glib code would be smaller, faster, more
> > readable, and consequently maintainable. Old code may
> > still ignore the return value, so that a change has only
> > minimal effect on existing code.