On Wed, Nov 02, 2016 at 10:52:35PM +0800, Christopher Li wrote:
> On Wed, Nov 2, 2016 at 8:48 PM, Luc Van Oostenryck
> <luc.vanoostenryck@xxxxxxxxx> wrote:
> >> + while (list->nr == 0)
> >> + list = list->prev;
>
> Yes, it can deed loop here when the whole list is empty.
>
> > FOR_EACH_PTR_REVERSE(list, ptr) {
> > DELETE_CURRENT_PTR(ptr);
> > last = last_ptr_list((struct ptr_list *)list);
> > } END_FOR_EACH_PTR_REVERSE(ptr);
> > }
> > loops forever on a list with 1 element with your patch.
> > Without your patch, last_ptr_list() return an invalid pointer instead of NULL.
> >
>
> I can see what is going on now. The while loop should
> check for the list->prev has been loop back to the tail of
> the list.
>
> Some thing like this should fix it, I haven't test it at all.
>
> list = tail = list->prev;
> while (list->nr == 0) {
> list = list->prev;
> if (list == tail)
> return NULL;
> }
>
> Chris
> --
Yes, something like this should do, indeed.
But by looking at the code, I think that 'first_ptr_list()'
is also not immunised against the same problem.
Also these two functions first_ & last_ptr_list() were
trivial ones but now they maybe become a bit too heavy to be inlined?
Or, maybe we should create a 'safe' version of those two, not inlined,
and which do the correct list walking?
Or maybe we need to have a macro like LAST_PTR() which do everything needed,
like DELETE_CURRENT_PTR() do?
I would vote for the safe version.
Luc
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
