On 2015-03-10 14:25, Tanu Kaskinen wrote:
On Fri, 2015-03-06 at 21:20 +0100, Peter Meerwald wrote:
On Thu, 5 Mar 2015, David Henningsson wrote:

In case SHM is full or disabled, audio data is sent through the
io/srbchannel. When this channel in turn gets full, memblocks
could previously be split up. This could lead to crashes in case
the split was on non-frame boundaries (in combination with full
memblock queues).

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=88452
Signed-off-by: David Henningsson <[email protected]>
---
  src/pulsecore/pstream.c | 67 +++++++++++++++++++++++--------------------------
  1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/src/pulsecore/pstream.c b/src/pulsecore/pstream.c
index b0ed5a7..8c05a87 100644
--- a/src/pulsecore/pstream.c
+++ b/src/pulsecore/pstream.c
@@ -682,6 +682,36 @@ fail:
      return -1;
  }
+static void memblock_complete(pa_pstream *p, struct pstream_read *re) {
+    size_t l;
+    pa_memchunk chunk;
+    int64_t offset;
+
+    if (!p->receive_memblock_callback)
+        return;
+
+    /* Is this memblock data? Then pass it to the user */
+    l = re->index - PA_PSTREAM_DESCRIPTOR_SIZE;
+    if (l <= 0)
l is an unsigned data type; the difference should not become negative and
the check should just be for (l == 0)
There are many places where this kind of redundant negativity checks are
done (I suppose Lennart likes that style), so for consistency reasons, I
wouldn't say that the check *should* be just for (l == 0). I certainly
wouldn't be against using (l == 0) either, though (I don't personally
like the style of using redundant negativity checks).

But that's maybe irrelevant anyway, because I think the whole check is
redundant. There is a check elsewhere that ensures that empty memblocks
are rejected, so l can never be zero.

Even if David prefers to keep the check "just in case", the comment
certainly needs editing. "Is this memblock data?" is a nonsensical
question here - of course it's memblock data, that should be obvious
from the function name.

Otherwise this patch looks good to me.


Thanks, pushed all three after removing the "if" and the comment. The third one's indentation was fixed up as well.

// David

_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to