Re: ilist.h is not useful as-is

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: ilist.h is not useful as-is
Date: 2013-07-24 21:50:52
Message-ID: 5558.1374702652@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> The first attached patch adds slist_delete_current(), updates the
> comments addressing your points and converts the bgworker code to pass
> the iterator around (it's more efficient which might actually matter
> with a few hundred bgworkers).

Applied with fixes. The slist_delete_current() logic wouldn't actually
work for deleting multiple adjacent list elements, because the foreach
macro would advance prev to point at the just-deleted element. Compare
the places where we use list_delete_cell() in a loop --- we're careful
to advance prev only when we *don't* delete the current element.
I dealt with that by making slist_delete_current() back up the
iterator's "cur" pointer to equal "prev", so that the loop macro's next
copying of "cur" to "prev" is a no-op. This means callers can't rely on
"cur" anymore after the deletion, but in typical usage they'd have
computed a pointer to their struct already so this shouldn't be a
problem. Another fine point is you can't use both slist_delete and
slist_delete_current within the same loop, since the former wouldn't
apply this correction.

Also, I fixed the slist_foreach_modify macro to not doubly evaluate
its lhead argument, since I think we agreed that was a bad idea,
and did some more work on the comments.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2013-07-24 23:02:11 Re: dynamic background workers, round two
Previous Message Andrew Gierth 2013-07-24 21:38:15 Re: Review: UNNEST (and other functions) WITH ORDINALITY