On Thu, Dec 1, 2016 at 1:26 PM, Tomas Vondra <tomas.von...@2ndquadrant.com> 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. > Moved to next CF with same status (needs review). Regards, Hari Babu Fujitsu Australia
