On 01/12/16 03:26, Tomas Vondra wrote: > > Dne 11/27/2016 v 11:02 PM Andres Freund napsal(a): >> On 2016-11-27 22:21:49 +0100, Petr Jelinek wrote: >>> On 27/11/16 21:47, Andres Freund wrote: >>>> Hi, >>>> >>>>>> +typedef struct SlabBlockData *SlabBlock; /* forward >>>>>> reference */ >>>>>> +typedef struct SlabChunkData *SlabChunk; >>>>>> >>>>>> Can we please not continue hiding pointers behind typedefs? It's a >>>>>> bad >>>>>> pattern, and that it's fairly widely used isn't a good excuse to >>>>>> introduce further usages of it. >>>>>> >>>>> >>>>> Why is it a bad pattern? >>>> >>>> It hides what is passed by reference, and what by value, and it >>>> makes it >>>> a guessing game whether you need -> or . since you don't know whether >>>> it's a pointer or the actual object. All to save a * in parameter and >>>> variable declaration?... >>>> >>> >>> FWIW I don't like that pattern either although it's used in many >>> parts of our code-base. >> >> But relatively few new ones, most of it is pretty old. >> > > I do agree it's not particularly pretty pattern, but in this case it's > fairly isolated in the mmgr sources, and I quite value the consistency > in this part of the code (i.e. that aset.c, slab.c and generation.c all > use the same approach). So I haven't changed this. > > The attached v7 fixes the off-by-one error in slab.c, causing failures > in test_decoding isolation tests, and renames Gen to Generation, as > proposed by Petr. >
I'd be happy with this patch now (as in committer ready) except that it does have some merge conflicts after the recent commits, so rebase is needed. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers