[asterisk-dev] watch out usages of AST_LIST_INSERT_TAIL

[Tony Mountifield]

In article <47012EC6.8060109 at digium.com>,
Jason Parker <jparker at digium.com> wrote:
> Luigi Rizzo wrote:
> > i just realised "the hard way" that AST_LIST_INSERT_TAIL can only
> > append a single element to a list, and it is not good for appending
> > two lists. The macro also assumes that the element has the 'tail'
> > pointer properly initialized.
> > 
> > I am not 100% sure that all places in asterisk where AST_LIST_INSERT_TAIL
> > is used honor the assumption.
> > Other than a scrutiny of all places where AST_LIST_INSERT_TAIL is used,
> > i would suggest a simple fix - changing the macro to handle the case
> > of list merging, as follows (more or less):
> > 
> >    #define AST_LIST_INSERT_TAIL(head, elm, field) do {                     \
> >       if (!(head)->first) {                                             \
> >                 (head)->first = (elm);                                  \
> >       } else {                                                          \
> >                 (head)->last->field.next = (elm);                       \
> >       }                                                                 \
> > 	(head)->last = (elm);                                   	\
> > 	while ((head)->last->field.next)		\
> > 		(head)->last = (head)->last->field.next;		\
> >    } while (0)
> > 
> > The extra overhead is negligible in the normal case
> > ((head)->last->field.next = NULL)
> > 
> > Of course, with all these pointer dereferences, one wonders if it
> > isn't better/safer to use the macro just as a frontend to a
> > function, using offsetof(field) to locate link field within the
> > structure.
> > 
> > cheers
> > luigi
> > 
> 
> Just a thought, but would it maybe be useful to have something like
> AST_LIST_COMBINE, to add the contents of list2 to list1?

You mean like AST_LIST_APPEND_LIST in linkedlists.h?

(added via bug 7802 in August 2006)

Cheers
Tony
-- 
Tony Mountifield
Work: tony at softins.co.uk - http://www.softins.co.uk
Play: tony at mountifield.org - http://tony.mountifield.org