On 2016-02-13 02:58, Ahmed S. Darwish wrote:
Hi!
On Fri, Feb 12, 2016 at 04:49:23PM +0100, David Henningsson wrote:
On 2016-02-12 01:14, Ahmed S. Darwish wrote:
+ /* Do not close file descriptors which are not our own */
+ if (type != PA_MEM_TYPE_SHARED_MEMFD)
+ pa_assert_se(pa_close(fd) == 0);
I believe the fd should be closed regardless of type?
Now you seem to leak an fd if type == PA_MEM_TYPE_SHARED_MEMFD?
Hmm, that comment should've been further clarified..
By "not close file descriptors which are not our own", I meant
not to close the fd behind our caller's back. It was just passed
to us as a parameter and thus we do not have any kind of
ownership to close it.
This is unlike attaching to a Posix SHM where the fd was created
by us in the same code path [ using shm_open() ] and thus we have
its complete ownership -- and must close it afterwards.
So for memfd attachment, the caller owns the passed fd and is
responsible for closing it when it sees fit. That's also why we
do not cache the passed fd in the callee and set pa_shm.memfd.fd
to -1.
Right, so if you would have instead written something like:
if (type == PA_MEM_TYPE_SHARED_MEMFD)
m->per_type.memfd.fd = fd;
else
pa_assert_se(pa_close(fd) == 0);
...it would be more obvious that the ownership of the fd is transferred
into the struct at that point, or closed.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
