[pulseaudio-commits] 4 commits - src/daemon src/pulse src/pulsecore src/tests src/utils

Peter Meerwald pmeerw at kemper.freedesktop.org
Sun Sep 4 21:06:24 UTC 2016


 src/daemon/main.c           |    2 +-
 src/pulse/sample.c          |   11 +++++------
 src/pulsecore/core-util.c   |   26 ++++++++++++++++++++++++--
 src/pulsecore/core-util.h   |   13 +++++++++++++
 src/pulsecore/macro.h       |   21 ---------------------
 src/pulsecore/memblock.c    |    5 +++--
 src/pulsecore/sample-util.c |    2 +-
 src/pulsecore/shm.c         |    7 ++++---
 src/pulsecore/sink-input.c  |    2 +-
 src/pulsecore/sink.c        |    2 +-
 src/tests/sigbus-test.c     |   13 +++++++------
 src/tests/stripnul.c        |    5 ++++-
 src/utils/padsp.c           |   11 +++++++++--
 13 files changed, 73 insertions(+), 47 deletions(-)

New commits:
commit 83f0a34ea6c6a6599207cacab29cced81e787001
Author: Peter Meerwald-Stadler <pmeerw at pmeerw.net>
Date:   Thu Aug 18 16:08:45 2016 +0200

    sample: Assert validity of sample_spec
    
    passing an invalid sample_spec to
    pa_sample_size_of_format(),
    pa_frame_size(),
    pa_bytes_per_second(),
    pa_bytes_to_usec(),
    pa_usec_to_bytes()
    currently gives a result of 0
    
    this is problematic as
    (a) it leads to many potential divide-by-zero issues flagged by Coverity,
    (b) pa_sample_spec_valid() is called often and the mostly unnecessary validation
    of the sample_spec cannot be optimized away due to pa_return_val_if_fail()
    (c) nobody checks the result for 0 and the behaviour is not documented
    
    this patch replaces pa_return_val_if_fail() with pa_assert()
    
    note that this commit changes the API!
    note that pa_return_val_if_fail() strangely logs an assertion, but then happily
    continues...
    
    fixes numerious CIDs

diff --git a/src/pulse/sample.c b/src/pulse/sample.c
index 1331c72..cb04254 100644
--- a/src/pulse/sample.c
+++ b/src/pulse/sample.c
@@ -56,37 +56,36 @@ size_t pa_sample_size_of_format(pa_sample_format_t f) {
 }
 
 size_t pa_sample_size(const pa_sample_spec *spec) {
-
     pa_assert(spec);
-    pa_return_val_if_fail(pa_sample_spec_valid(spec), 0);
+    pa_assert(pa_sample_spec_valid(spec));
 
     return size_table[spec->format];
 }
 
 size_t pa_frame_size(const pa_sample_spec *spec) {
     pa_assert(spec);
-    pa_return_val_if_fail(pa_sample_spec_valid(spec), 0);
+    pa_assert(pa_sample_spec_valid(spec));
 
     return size_table[spec->format] * spec->channels;
 }
 
 size_t pa_bytes_per_second(const pa_sample_spec *spec) {
     pa_assert(spec);
-    pa_return_val_if_fail(pa_sample_spec_valid(spec), 0);
+    pa_assert(pa_sample_spec_valid(spec));
 
     return spec->rate * size_table[spec->format] * spec->channels;
 }
 
 pa_usec_t pa_bytes_to_usec(uint64_t length, const pa_sample_spec *spec) {
     pa_assert(spec);
-    pa_return_val_if_fail(pa_sample_spec_valid(spec), 0);
+    pa_assert(pa_sample_spec_valid(spec));
 
     return (((pa_usec_t) (length / (size_table[spec->format] * spec->channels)) * PA_USEC_PER_SEC) / spec->rate);
 }
 
 size_t pa_usec_to_bytes(pa_usec_t t, const pa_sample_spec *spec) {
     pa_assert(spec);
-    pa_return_val_if_fail(pa_sample_spec_valid(spec), 0);
+    pa_assert(pa_sample_spec_valid(spec));
 
     return (size_t) (((t * spec->rate) / PA_USEC_PER_SEC)) * (size_table[spec->format] * spec->channels);
 }

commit 250fd43bdc9f45bda071f38bb2420ace31b48f81
Author: Peter Meerwald-Stadler <pmeerw at pmeerw.net>
Date:   Thu Aug 18 09:24:41 2016 +0200

    tests: Assert granularity range in stripnul.c
    
    granularity must not be larger than buffer size
    
    CID 1138482

diff --git a/src/tests/stripnul.c b/src/tests/stripnul.c
index 75390bd..e4b07aa 100644
--- a/src/tests/stripnul.c
+++ b/src/tests/stripnul.c
@@ -26,6 +26,8 @@
 #include <pulse/xmalloc.h>
 #include <pulsecore/macro.h>
 
+#define MAX_BUFFER (16*1024)
+
 int main(int argc, char *argv[]) {
     FILE *i, *o;
     size_t granularity;
@@ -34,13 +36,14 @@ int main(int argc, char *argv[]) {
 
     pa_assert_se(argc >= 2);
     pa_assert_se((granularity = (size_t) atoi(argv[1])) >= 1);
+    pa_assert(granularity <= MAX_BUFFER);
     pa_assert_se((i = (argc >= 3) ? fopen(argv[2], "r") : stdin));
     pa_assert_se((o = (argc >= 4) ? fopen(argv[3], "w") : stdout));
 
     zero = pa_xmalloc0(granularity);
 
     for (;;) {
-        uint8_t buffer[16*1024], *p;
+        uint8_t buffer[MAX_BUFFER], *p;
         size_t k;
 
         k = fread(buffer, granularity, sizeof(buffer)/granularity, i);

commit 45d9030638fb0900d2b18f2d5c009e6076ee3fe8
Author: Peter Meerwald-Stadler <pmeerw at pmeerw.net>
Date:   Thu Aug 18 01:06:47 2016 +0200

    core: Replace PA_PAGE_SIZE with pa_page_size()
    
    PA_PAGE_SIZE using sysconf() may return a negative number
    
    CID 1137925, CID 1137926, CID 1138485
    
    instead of calling sysconf() directly, add function pa_page_size()
    which uses the guestimate 4096 in case sysconf(_SC_PAGE_SIZE) fails
    
    using PA_ONCE to only evaluate sysconf() once

diff --git a/src/daemon/main.c b/src/daemon/main.c
index a313dc5..dc5e5bc 100644
--- a/src/daemon/main.c
+++ b/src/daemon/main.c
@@ -918,7 +918,7 @@ int main(int argc, char *argv[]) {
 
     pa_log_debug("Found %u CPUs.", pa_ncpus());
 
-    pa_log_info("Page size is %lu bytes", (unsigned long) PA_PAGE_SIZE);
+    pa_log_info("Page size is %zu bytes", pa_page_size());
 
 #ifdef HAVE_VALGRIND_MEMCHECK_H
     pa_log_debug("Compiled with Valgrind support: yes");
diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c
index da1b0c8..350f35b 100644
--- a/src/pulsecore/core-util.c
+++ b/src/pulsecore/core-util.c
@@ -138,6 +138,7 @@
 #include <pulsecore/strlist.h>
 #include <pulsecore/cpu-x86.h>
 #include <pulsecore/pipe.h>
+#include <pulsecore/once.h>
 
 #include "core-util.h"
 
@@ -2553,6 +2554,7 @@ void *pa_will_need(const void *p, size_t l) {
     size_t size;
     int r = ENOTSUP;
     size_t bs;
+    const size_t page_size = pa_page_size();
 
     pa_assert(p);
     pa_assert(l > 0);
@@ -2577,7 +2579,7 @@ void *pa_will_need(const void *p, size_t l) {
 #ifdef RLIMIT_MEMLOCK
     pa_assert_se(getrlimit(RLIMIT_MEMLOCK, &rlim) == 0);
 
-    if (rlim.rlim_cur < PA_PAGE_SIZE) {
+    if (rlim.rlim_cur < page_size) {
         pa_log_debug("posix_madvise() failed (or doesn't exist), resource limits don't allow mlock(), can't page in data: %s", pa_cstrerror(r));
         errno = EPERM;
         return (void*) p;
@@ -2585,7 +2587,7 @@ void *pa_will_need(const void *p, size_t l) {
 
     bs = PA_PAGE_ALIGN((size_t) rlim.rlim_cur);
 #else
-    bs = PA_PAGE_SIZE*4;
+    bs = page_size*4;
 #endif
 
     pa_log_debug("posix_madvise() failed (or doesn't exist), trying mlock(): %s", pa_cstrerror(r));
@@ -3669,3 +3671,23 @@ bool pa_running_in_vm(void) {
 
     return false;
 }
+
+size_t pa_page_size(void) {
+#if defined(PAGE_SIZE)
+    return PAGE_SIZE;
+#elif defined(PAGESIZE)
+    return PAGESIZE;
+#elif defined(HAVE_SYSCONF)
+    static size_t page_size = 4096; /* Let's hope it's like x86. */
+
+    PA_ONCE_BEGIN {
+        long ret = sysconf(_SC_PAGE_SIZE);
+        if (ret > 0)
+            page_size = ret;
+    } PA_ONCE_END;
+
+    return page_size;
+#else
+    return 4096;
+#endif
+}
diff --git a/src/pulsecore/core-util.h b/src/pulsecore/core-util.h
index 5725ca7..ede1ae2 100644
--- a/src/pulsecore/core-util.h
+++ b/src/pulsecore/core-util.h
@@ -302,4 +302,17 @@ bool pa_running_in_vm(void);
 char *pa_win32_get_toplevel(HANDLE handle);
 #endif
 
+size_t pa_page_size(void);
+
+/* Rounds down */
+static inline void* PA_PAGE_ALIGN_PTR(const void *p) {
+    return (void*) (((size_t) p) & ~(pa_page_size() - 1));
+}
+
+/* Rounds up */
+static inline size_t PA_PAGE_ALIGN(size_t l) {
+    size_t page_size = pa_page_size();
+    return (l + page_size - 1) & ~(page_size - 1);
+}
+
 #endif
diff --git a/src/pulsecore/macro.h b/src/pulsecore/macro.h
index f80b43c..2c5d5f2 100644
--- a/src/pulsecore/macro.h
+++ b/src/pulsecore/macro.h
@@ -44,17 +44,6 @@
 #endif
 #endif
 
-#if defined(PAGE_SIZE)
-#define PA_PAGE_SIZE ((size_t) PAGE_SIZE)
-#elif defined(PAGESIZE)
-#define PA_PAGE_SIZE ((size_t) PAGESIZE)
-#elif defined(HAVE_SYSCONF)
-#define PA_PAGE_SIZE ((size_t) (sysconf(_SC_PAGE_SIZE)))
-#else
-/* Let's hope it's like x86. */
-#define PA_PAGE_SIZE ((size_t) 4096)
-#endif
-
 /* Rounds down */
 static inline void* PA_ALIGN_PTR(const void *p) {
     return (void*) (((size_t) p) & ~(sizeof(void*) - 1));
@@ -65,16 +54,6 @@ static inline size_t PA_ALIGN(size_t l) {
     return ((l + sizeof(void*) - 1) & ~(sizeof(void*) - 1));
 }
 
-/* Rounds down */
-static inline void* PA_PAGE_ALIGN_PTR(const void *p) {
-    return (void*) (((size_t) p) & ~(PA_PAGE_SIZE - 1));
-}
-
-/* Rounds up */
-static inline size_t PA_PAGE_ALIGN(size_t l) {
-    return (l + PA_PAGE_SIZE - 1) & ~(PA_PAGE_SIZE - 1);
-}
-
 #if defined(__GNUC__)
     #define PA_UNUSED __attribute__ ((unused))
 #else
diff --git a/src/pulsecore/memblock.c b/src/pulsecore/memblock.c
index 17520ed..dbad32a 100644
--- a/src/pulsecore/memblock.c
+++ b/src/pulsecore/memblock.c
@@ -828,13 +828,14 @@ static void memblock_replace_import(pa_memblock *b) {
 pa_mempool *pa_mempool_new(pa_mem_type_t type, size_t size, bool per_client) {
     pa_mempool *p;
     char t1[PA_BYTES_SNPRINT_MAX], t2[PA_BYTES_SNPRINT_MAX];
+    const size_t page_size = pa_page_size();
 
     p = pa_xnew0(pa_mempool, 1);
     PA_REFCNT_INIT(p);
 
     p->block_size = PA_PAGE_ALIGN(PA_MEMPOOL_SLOT_SIZE);
-    if (p->block_size < PA_PAGE_SIZE)
-        p->block_size = PA_PAGE_SIZE;
+    if (p->block_size < page_size)
+        p->block_size = page_size;
 
     if (size <= 0)
         p->n_blocks = PA_MEMPOOL_SLOTS_MAX;
diff --git a/src/pulsecore/sample-util.c b/src/pulsecore/sample-util.c
index 8ae5db9..b2a28eb 100644
--- a/src/pulsecore/sample-util.c
+++ b/src/pulsecore/sample-util.c
@@ -39,7 +39,7 @@
 
 #include "sample-util.h"
 
-#define PA_SILENCE_MAX (PA_PAGE_SIZE*16)
+#define PA_SILENCE_MAX (pa_page_size()*16)
 
 pa_memblock *pa_silence_memblock(pa_memblock* b, const pa_sample_spec *spec) {
     void *data;
diff --git a/src/pulsecore/shm.c b/src/pulsecore/shm.c
index 1c5eb44..919d71a 100644
--- a/src/pulsecore/shm.c
+++ b/src/pulsecore/shm.c
@@ -126,7 +126,7 @@ static int privatemem_create(pa_shm *m, size_t size) {
     {
         int r;
 
-        if ((r = posix_memalign(&m->ptr, PA_PAGE_SIZE, size)) < 0) {
+        if ((r = posix_memalign(&m->ptr, pa_page_size(), size)) < 0) {
             pa_log("posix_memalign() failed: %s", pa_cstrerror(r));
             return r;
         }
@@ -300,6 +300,7 @@ finish:
 void pa_shm_punch(pa_shm *m, size_t offset, size_t size) {
     void *ptr;
     size_t o;
+    const size_t page_size = pa_page_size();
 
     pa_assert(m);
     pa_assert(m->ptr);
@@ -318,13 +319,13 @@ void pa_shm_punch(pa_shm *m, size_t offset, size_t size) {
     o = (size_t) ((uint8_t*) ptr - (uint8_t*) PA_PAGE_ALIGN_PTR(ptr));
 
     if (o > 0) {
-        size_t delta = PA_PAGE_SIZE - o;
+        size_t delta = page_size - o;
         ptr = (uint8_t*) ptr + delta;
         size -= delta;
     }
 
     /* Align the size down to multiples of page size */
-    size = (size / PA_PAGE_SIZE) * PA_PAGE_SIZE;
+    size = (size / page_size) * page_size;
 
 #ifdef MADV_REMOVE
     if (madvise(ptr, size, MADV_REMOVE) >= 0)
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 435e63e..0dda204 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -44,7 +44,7 @@
 /* #define SINK_INPUT_DEBUG */
 
 #define MEMBLOCKQ_MAXLENGTH (32*1024*1024)
-#define CONVERT_BUFFER_LENGTH (PA_PAGE_SIZE)
+#define CONVERT_BUFFER_LENGTH (pa_page_size())
 
 PA_DEFINE_PUBLIC_CLASS(pa_sink_input, pa_msgobject);
 
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index a41a78a..5c6a9c6 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -50,7 +50,7 @@
 #include "sink.h"
 
 #define MAX_MIX_CHANNELS 32
-#define MIX_BUFFER_LENGTH (PA_PAGE_SIZE)
+#define MIX_BUFFER_LENGTH (pa_page_size())
 #define ABSOLUTE_MIN_LATENCY (500)
 #define ABSOLUTE_MAX_LATENCY (10*PA_USEC_PER_SEC)
 #define DEFAULT_FIXED_LATENCY (250*PA_USEC_PER_MSEC)
diff --git a/src/tests/sigbus-test.c b/src/tests/sigbus-test.c
index 8a660ee..dbfc3d1 100644
--- a/src/tests/sigbus-test.c
+++ b/src/tests/sigbus-test.c
@@ -33,6 +33,7 @@ START_TEST (sigbus_test) {
     void *p;
     int fd;
     pa_memtrap *m;
+    const size_t page_size = pa_page_size();
 
     pa_log_set_level(PA_LOG_DEBUG);
     pa_memtrap_install();
@@ -40,14 +41,14 @@ START_TEST (sigbus_test) {
     /* Create the memory map */
     fail_unless((fd = open("sigbus-test-map", O_RDWR|O_TRUNC|O_CREAT, 0660)) >= 0);
     fail_unless(unlink("sigbus-test-map") == 0);
-    fail_unless(ftruncate(fd, PA_PAGE_SIZE) >= 0);
-    fail_unless((p = mmap(NULL, PA_PAGE_SIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0)) != MAP_FAILED);
+    fail_unless(ftruncate(fd, page_size) >= 0);
+    fail_unless((p = mmap(NULL, page_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0)) != MAP_FAILED);
 
     /* Register memory map */
-    m = pa_memtrap_add(p, PA_PAGE_SIZE);
+    m = pa_memtrap_add(p, page_size);
 
     /* Use memory map */
-    pa_snprintf(p, PA_PAGE_SIZE, "This is a test that should work fine.");
+    pa_snprintf(p, page_size, "This is a test that should work fine.");
 
     /* Verify memory map */
     pa_log("Let's see if this worked: %s", (char*) p);
@@ -58,7 +59,7 @@ START_TEST (sigbus_test) {
     fail_unless(ftruncate(fd, 0) >= 0);
 
     /* Use memory map */
-    pa_snprintf(p, PA_PAGE_SIZE, "This is a test that should fail but get caught.");
+    pa_snprintf(p, page_size, "This is a test that should fail but get caught.");
 
     /* Verify memory map */
     pa_log("Let's see if this worked: %s", (char*) p);
@@ -66,7 +67,7 @@ START_TEST (sigbus_test) {
     fail_unless(pa_memtrap_is_good(m) == false);
 
     pa_memtrap_remove(m);
-    munmap(p, PA_PAGE_SIZE);
+    munmap(p, page_size);
     close(fd);
 }
 END_TEST

commit c99efbffd694ff2d2088339e8412972d90e7cd95
Author: Peter Meerwald-Stadler <pmeerw at pmeerw.net>
Date:   Wed Aug 17 23:52:17 2016 +0200

    padsp: Fix flush and improve error handling
    
    read() can return a number of bytes read less than k
    
    in addition, handle EAGAIN and EOF
    
    CID 1137981

diff --git a/src/utils/padsp.c b/src/utils/padsp.c
index 943479b..7a94114 100644
--- a/src/utils/padsp.c
+++ b/src/utils/padsp.c
@@ -1768,11 +1768,18 @@ static int dsp_flush_fd(int fd) {
     while (l > 0) {
         char buf[1024];
         size_t k;
+        ssize_t r;
 
         k = (size_t) l > sizeof(buf) ? sizeof(buf) : (size_t) l;
-        if (read(fd, buf, k) < 0)
+        r = read(fd, buf, k);
+        if (r < 0) {
+            if (errno == EAGAIN)
+                break;
             debug(DEBUG_LEVEL_NORMAL, __FILE__": read(): %s\n", strerror(errno));
-        l -= k;
+            return -1;
+        } else if (r == 0)
+            break;
+        l -= r;
     }
 
     return 0;



More information about the pulseaudio-commits mailing list