On 2016-02-19 20:28, Ahmed S. Darwish wrote:
> On Fri, Feb 19, 2016 at 03:54:06PM +0100, David Henningsson wrote:
>>
>> On 2016-02-16 22:41, Ahmed S. Darwish wrote:
>>> Hi :-)
>>>
> ...
>>> 4. The code above is actually for the PA endpoint creating a
>>> memfd mempool and registering it with the other side. That
>>> is, pa_mempool_take_memfd_fd() is just used when registering
>>> a memfd mempool over the pstream.
>>
>> Ah, so that's only used on the sending side. Maybe I was just
>> confused when I wrote the above.
>>
>> But then I have a follow-up question. In shm_attach, which is on the
>> receiving side, why do we need to save the fd in pa_shm at all? I
>> mean, we don't need to send it to anyone else (right?), so all we
>> need is the memory and mmap keeps that open for us. So, we could
>> just close the memfd in shm_attach?
>
> Yup, this is what exactly happens. No fd is ever cached on the
> receiving side at shm_attch(). Similar point was raised in patch #5
> review and a more detailed reply was posted here [*] -- in the 2nd
> reply hunk.
>
> Given that this issue is now raised twice, I'm sure the code is
> badly unclear, possibly due to the per-type stuff now removed.
>
> Anyway here's how shm_attch() now looks like in v3:
>
> static int shm_attach(pa_shm *m, pa_mem_type_t type, unsigned
> id, int memfd_fd, ..) {
> ..
> /* In case of attaching to memfd areas, _the caller_
> * maintains ownership of the passed fd and has the sole
> * responsibility of closing it down.. For other types, we
> * are the code path which created the fd in the first
> * place and we're thus the ones responsible for closing it
> * down */
> if (type != PA_MEM_TYPE_SHARED_MEMFD)
> pa_assert_se(pa_close(fd) == 0);
>
> m->type = type;
> m->id = id;
> m->size = (size_t) st.st_size;
> m->do_unlink = false;
> m->fd = -1;
> }
>
> I guess this is now much more clear :-)
Okay, so it's mostly a design decision then? Whether to take ownership
of the memfd and thus close it down when we're done (and simpler code as
a result?), or let the caller do the same right after the call to
shm_attach?
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
