[pulseaudio-discuss] [PATCH v2 04/12] pulsecore: Split pa_shm mempool backend into pa_shm and pa_privatemem

[Ahmed S. Darwish]

On Sat, Feb 13, 2016 at 12:49:00PM +0200, Ahmed S. Darwish wrote:
> On Fri, Feb 12, 2016 at 03:25:19PM +0100, David Henningsson wrote:
> >
...
> > So, this part I'm still quite hesitant about. It would be easier just to
> > change pa_shm to look like this, which is similar to I suggested earlier
> > [1]:
> > 
> > struct pa_shm {
> >   void *ptr;
> >   size_t size;
> >   pa_mem_type_t type;
> >   /* The three fields below are unused if type is PA_MEM_TYPE_PRIVATE */
> >   int fd;
> >   unsigned id;
> >   bool do_unlink; /* Used for PA_MEM_TYPE_SHARED_POSIX only */
> > }
> >
> > Instead you did a massive refactoring, add unions inside other unions, and a
> > privatemem type which contains just one other field, and even has its own
> > files? Sorry, but I don't see the point in doing all of that. (Maybe it
> > would make sense in an OOP language, but I don't think it does in C.)
> >
> 
> OK, it's clear you don't like this. Let's tackle the critique
> above in two parts.
> 
> First, regarding the 'per_type' construct...
>
...
> 
> In our case it gives _a clear warning_ that we are accessing
> parts only related to posix SHM or memfds.
> 
> Tastes differ, but a 'per_type' is more verbose and clear than
> putting /*This is only for memfds*/, /*This is only for posix*/
> comments over different shm elements. That is even way more true
> on the caller side of the equation where such markings won't be
> visible.
>

Well, OK, I've just finished a v3 without the per_type union in
'pa_shm' and things are not that different.

So no problem on my side, it's now removed...

Thanks,

-- 
Darwish
http://darwish.chasingpointers.com