12.09.2014 14:10, David Henningsson wrote:
If the keyboard is unplugged, it looks like the kernel is reporting
back -ENODEV when trying to close the fd. This is probably a kernel
error, but still, it's better to complain than to crash.
Buglink: https://bugs.freedesktop.org/show_bug.cgi?id=80867
Reported-by: Stelios Bounanos
Signed-off-by: David Henningsson <[email protected]>
---
src/modules/module-mmkbd-evdev.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
The patch neither looks definitely good nor looks definitely bad to me
(but hey, it fixes a crash!). I have verified that there are no other
instances of pa_close in the same module (i.e. that could attempt to
close the same device and thus would need a similar quirk), but am not
sure whether the "log the close() failure" logic belongs here or should
be done generally in pa_close().
I have also looked at the definition of pa_close(). While it is
unrelated to the problem that the patch is trying to solve, I must say
that either any pa_assert_se(pa_close(fd) == 0) in any file is a bug, or
the loop that retries on EINTR inside pa_close() is a bug, or both. The
retry-loop is a bug at least for Linux. Here is why.
https://lwn.net/Articles/576478/
https://lwn.net/Articles/576591/
Even for the hypothetical situation where close() can fail with EINTR,
see the manual page:
In particular close() should not be retried after an EINTR since this
may cause a reused descriptor from another thread to be closed.
But it is retried. The worst case is already described above, and the
best case is that it, when retried, fails with EBADFD and causes the
assert to fail.
diff --git a/src/modules/module-mmkbd-evdev.c b/src/modules/module-mmkbd-evdev.c
index 9ab7eb9..0b04f0f 100644
--- a/src/modules/module-mmkbd-evdev.c
+++ b/src/modules/module-mmkbd-evdev.c
@@ -256,8 +256,11 @@ void pa__done(pa_module*m) {
if (u->io)
m->core->mainloop->io_free(u->io);
- if (u->fd >= 0)
- pa_assert_se(pa_close(u->fd) == 0);
+ if (u->fd >= 0) {
+ int r = pa_close(u->fd);
+ if (r < 0) /* https://bugs.freedesktop.org/show_bug.cgi?id=80867 */
+ pa_log("Closing fd failed: %s", pa_cstrerror(errno));
+ }
pa_xfree(u->sink_name);
pa_xfree(u);
--
Alexander E. Patrakov
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
