Re: embedded list v2

On Friday, September 14, 2012 10:57:54 PM Tom Lane wrote:
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> > One thing I would like more input in, is whether people think it's
> > worthwhile to split dlists and slists into separate files. Thus far
> > this has been mentioned by three people independently.
>
> They're small enough and similar enough that one header and one .c file
> seem like plenty; but I don't really have a strong opinion about it.
>
> > Another question is whether ilist_container() should actually be a more
> > general macro "containerof" defined in c.h. (ISTM it would be necessary
> > to have this macro if we want to split into two files; that way we don't
> > need to have two macros dlist_container and slist_container with
> > identical definition, or alternatively a third file that defines just
> > ilist_container)
>
> I'd vote for not having that at all, but rather two separate macros
> dlist_container and slist_container. If we had a bunch of operations
> that could work interchangeably on dlists and slists, it might be worth
> having a concept of "ilist" --- but if we only have this, it would be
> better to remove the concept from the API altogether.
>
> > Third question is about the INLINE_IF_POSSIBLE business as commented by
> > Peter. It seems to me that the simple technique used here to avoid
> > having two copies of the source could be used by memcxt.c, list.c,
> > sortsupport.c as well (maybe clean up fastgetattr too).
>
> Yeah, looks reasonable ... material for a different patch of course.
> But that would mean INLINE_IF_POSSIBLE should be defined someplace else,
> perhaps c.h. Also, I'm not that thrilled with having the header file
> define ILIST_USE_DEFINITION. I suggest that it might be better to do
>
> #if defined(USE_INLINE) || defined(DEFINE_ILIST_FUNCTIONS)
> ... function decls here ...
> #else
> ... extern decls here ...
> #endif
>
> where ilist.c defines DEFINE_ILIST_FUNCTIONS before including the
> header.
I am preparing a new version of this right now. So, some last ditch questions
are coming up...

The reason I had the header declare DEFINE_ILIST_FUNCTIONS (or rather
ILIST_USE_DEFINITION back then) instead of reusing USE_INLINE directly is that
it makes it easier to locally change a "module" to not inlining which makes
testing the !USE_INLINE case easier. Does anybody think this is worth
something? I have no strong feelings but found it convenient.

Greetings,

Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services