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
