On 2016-02-12 01:12, Ahmed S. Darwish wrote:
Mempools use "pa_shm" for their pools memory allocation. pa_shm is
then overloaded with two unrelated tasks: either allocating the pool
using posix SHM or allocating the same pool using private malloc()s.
The choice depends on system configuration and abilities.

This will not scale when a third pool memory allocation mechanism
is added (memfds) . Thus, split pa_shm responsibility into:

   - pa_shm: responsible for all *shared* memory, posix and memfds
   - pa_privatemem: only responsible for private memory allocations

This way, memfd support can easily be added later.

So, this part I'm still quite hesitant about. It would be easier just to change pa_shm to look like this, which is similar to I suggested earlier [1]:

struct pa_shm {
  void *ptr;
  size_t size;
  pa_mem_type_t type;
  /* The three fields below are unused if type is PA_MEM_TYPE_PRIVATE */
  int fd;
  unsigned id;
  bool do_unlink; /* Used for PA_MEM_TYPE_SHARED_POSIX only */
}

Instead you did a massive refactoring, add unions inside other unions, and a privatemem type which contains just one other field, and even has its own files? Sorry, but I don't see the point in doing all of that. (Maybe it would make sense in an OOP language, but I don't think it does in C.)

[1] https://lists.freedesktop.org/archives/pulseaudio-discuss/2015-September/024482.html


Signed-off-by: Ahmed S. Darwish <[email protected]>
---
  src/Makefile.am            |   1 +
  src/pulsecore/mem.h        |   9 +++
  src/pulsecore/memblock.c   |  68 ++++++++++++---------
  src/pulsecore/privatemem.c |  78 ++++++++++++++++++++++++
  src/pulsecore/privatemem.h |  35 +++++++++++
  src/pulsecore/shm.c        | 148 ++++++++++++++++-----------------------------
  src/pulsecore/shm.h        |  11 ++--
  7 files changed, 220 insertions(+), 130 deletions(-)
  create mode 100644 src/pulsecore/privatemem.c
  create mode 100644 src/pulsecore/privatemem.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 6cb5bd7..f3a62fb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -701,6 +701,7 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \
                pulsecore/sample-util.c pulsecore/sample-util.h \
                pulsecore/mem.h \
                pulsecore/shm.c pulsecore/shm.h \
+               pulsecore/privatemem.c pulsecore/privatemem.h \
                pulsecore/bitset.c pulsecore/bitset.h \
                pulsecore/socket-client.c pulsecore/socket-client.h \
                pulsecore/socket-server.c pulsecore/socket-server.h \
diff --git a/src/pulsecore/mem.h b/src/pulsecore/mem.h
index b6fdba6..d0a063c 100644
--- a/src/pulsecore/mem.h
+++ b/src/pulsecore/mem.h
@@ -24,6 +24,15 @@

  #include <pulsecore/macro.h>

+/* Generic interface representing a plain memory area and its size.
+ *
+ * Different memory backends (posix SHM, private, etc.) embed this
+ * structure as their first element for polymorphic access. */
+typedef struct pa_mem {
+    void *ptr;
+    size_t size;
+} pa_mem;
+
  typedef enum pa_mem_type {
      PA_MEM_TYPE_SHARED_POSIX,         /* Data is shared and created using 
POSIX shm_open() */
      PA_MEM_TYPE_SHARED_MEMFD,         /* Data is shared and created using 
Linux memfd_create() */
diff --git a/src/pulsecore/memblock.c b/src/pulsecore/memblock.c
index 3a33a6f..154bd67 100644
--- a/src/pulsecore/memblock.c
+++ b/src/pulsecore/memblock.c
@@ -36,7 +36,9 @@
  #include <pulse/xmalloc.h>
  #include <pulse/def.h>

+#include <pulsecore/mem.h>
  #include <pulsecore/shm.h>
+#include <pulsecore/privatemem.h>
  #include <pulsecore/log.h>
  #include <pulsecore/hashmap.h>
  #include <pulsecore/semaphore.h>
@@ -145,8 +147,14 @@ struct pa_mempool {
      pa_semaphore *semaphore;
      pa_mutex *mutex;

-    pa_mem_type_t type;
-    pa_shm memory;
+    bool shared;
+    union {
+        pa_mem memory;
+        union {
+            pa_shm shm;
+            pa_privatemem privatemem;
+        } per_type;
+    };

      size_t block_size;
      unsigned n_blocks;
@@ -748,11 +756,9 @@ static void memblock_replace_import(pa_memblock *b) {

  pa_mempool *pa_mempool_new(pa_mem_type_t type, size_t size) {
      pa_mempool *p;
-    bool shared;
      char t1[PA_BYTES_SNPRINT_MAX], t2[PA_BYTES_SNPRINT_MAX];

      p = pa_xnew0(pa_mempool, 1);
-    p->type = type;

      p->block_size = PA_PAGE_ALIGN(PA_MEMPOOL_SLOT_SIZE);
      if (p->block_size < PA_PAGE_SIZE)
@@ -767,10 +773,13 @@ pa_mempool *pa_mempool_new(pa_mem_type_t type, size_t 
size) {
              p->n_blocks = 2;
      }

-    shared = pa_mem_type_is_shared(type);
-    if (pa_shm_create_rw(&p->memory, p->n_blocks * p->block_size, shared, 0700) 
< 0) {
-        pa_xfree(p);
-        return NULL;
+    if (pa_mem_type_is_shared(type)) {
+        p->shared = true;
+        if (pa_shm_create(&p->per_type.shm, type, p->n_blocks * p->block_size, 
0700) < 0)
+            goto mem_create_fail;
+    } else {
+        if (pa_privatemem_create(&p->per_type.privatemem, p->n_blocks * 
p->block_size) < 0)
+            goto mem_create_fail;
      }

      pa_log_debug("Using %s memory pool with %u slots of size %s each, total size 
is %s, maximum usable slot size is %lu",
@@ -791,6 +800,10 @@ pa_mempool *pa_mempool_new(pa_mem_type_t type, size_t 
size) {
      p->free_slots = pa_flist_new(p->n_blocks);

      return p;
+
+mem_create_fail:
+    pa_xfree(p);
+    return NULL;
  }

  void pa_mempool_free(pa_mempool *p) {
@@ -852,15 +865,10 @@ void pa_mempool_free(pa_mempool *p) {
  /*         PA_DEBUG_TRAP; */
      }

-    switch (p->type) {
-    case PA_MEM_TYPE_SHARED_POSIX:
-    case PA_MEM_TYPE_PRIVATE:
-        pa_shm_free(&p->memory);
-        break;
-    case PA_MEM_TYPE_SHARED_MEMFD:
-        pa_assert_not_reached();
-        break;
-    }
+    if (pa_mempool_is_shared(p))
+        pa_shm_free(&p->per_type.shm);
+    else
+        pa_privatemem_free(&p->per_type.privatemem);

      pa_mutex_free(p->mutex);
      pa_semaphore_free(p->semaphore);
@@ -896,7 +904,7 @@ void pa_mempool_vacuum(pa_mempool *p) {
              ;

      while ((slot = pa_flist_pop(list))) {
-        pa_shm_punch(&p->memory, (size_t) ((uint8_t*) slot - (uint8_t*) 
p->memory.ptr), p->block_size);
+        pa_shm_punch(&p->per_type.shm, (size_t) ((uint8_t*) slot - (uint8_t*) 
p->memory.ptr), p->block_size);

          while (pa_flist_push(p->free_slots, slot))
              ;
@@ -909,10 +917,10 @@ void pa_mempool_vacuum(pa_mempool *p) {
  int pa_mempool_get_shm_id(pa_mempool *p, uint32_t *id) {
      pa_assert(p);

-    if (!p->memory.shared)
+    if (!pa_mempool_is_shared(p))
          return -1;

-    *id = p->memory.id;
+    *id = p->per_type.shm.id;

      return 0;
  }
@@ -921,7 +929,7 @@ int pa_mempool_get_shm_id(pa_mempool *p, uint32_t *id) {
  bool pa_mempool_is_shared(pa_mempool *p) {
      pa_assert(p);

-    return p->memory.shared;
+    return p->shared;
  }

  /* For receiving blocks from other nodes */
@@ -964,7 +972,7 @@ static pa_memimport_segment* segment_attach(pa_memimport 
*i, uint32_t shm_id, bo

      seg->writable = writable;
      seg->import = i;
-    seg->trap = pa_memtrap_add(seg->memory.ptr, seg->memory.size);
+    seg->trap = pa_memtrap_add(seg->memory.mem.ptr, seg->memory.mem.size);

      pa_hashmap_put(i->segments, PA_UINT32_TO_PTR(seg->memory.id), seg);
      return seg;
@@ -1044,7 +1052,7 @@ pa_memblock* pa_memimport_get(pa_memimport *i, uint32_t 
block_id, uint32_t shm_i
          goto finish;
      }

-    if (offset+size > seg->memory.size)
+    if (offset+size > seg->memory.mem.size)
          goto finish;

      if (!(b = pa_flist_pop(PA_STATIC_FLIST_GET(unused_memblocks))))
@@ -1055,7 +1063,7 @@ pa_memblock* pa_memimport_get(pa_memimport *i, uint32_t 
block_id, uint32_t shm_i
      b->type = PA_MEMBLOCK_IMPORTED;
      b->read_only = !writable;
      b->is_silence = false;
-    pa_atomic_ptr_store(&b->data, (uint8_t*) seg->memory.ptr + offset);
+    pa_atomic_ptr_store(&b->data, (uint8_t*) seg->memory.mem.ptr + offset);
      b->length = size;
      pa_atomic_store(&b->n_acquired, 0);
      pa_atomic_store(&b->please_signal, 0);
@@ -1103,7 +1111,7 @@ pa_memexport* pa_memexport_new(pa_mempool *p, 
pa_memexport_revoke_cb_t cb, void
      pa_assert(p);
      pa_assert(cb);

-    if (!p->memory.shared)
+    if (!pa_mempool_is_shared(p))
          return NULL;

      e = pa_xnew(pa_memexport, 1);
@@ -1231,7 +1239,7 @@ static pa_memblock *memblock_shared_copy(pa_mempool *p, 
pa_memblock *b) {

  /* Self-locked */
  int pa_memexport_put(pa_memexport *e, pa_memblock *b, uint32_t *block_id, 
uint32_t *shm_id, size_t *offset, size_t * size) {
-    pa_shm *memory;
+    pa_shm  *memory;
      struct memexport_slot *slot;
      void *data;

@@ -1274,14 +1282,14 @@ int pa_memexport_put(pa_memexport *e, pa_memblock *b, 
uint32_t *block_id, uint32
      } else {
          pa_assert(b->type == PA_MEMBLOCK_POOL || b->type == 
PA_MEMBLOCK_POOL_EXTERNAL);
          pa_assert(b->pool);
-        memory = &b->pool->memory;
+        memory = &b->pool->per_type.shm;
      }

-    pa_assert(data >= memory->ptr);
-    pa_assert((uint8_t*) data + b->length <= (uint8_t*) memory->ptr + 
memory->size);
+    pa_assert(data >= memory->mem.ptr);
+    pa_assert((uint8_t*) data + b->length <= (uint8_t*) memory->mem.ptr + 
memory->mem.size);

      *shm_id = memory->id;
-    *offset = (size_t) ((uint8_t*) data - (uint8_t*) memory->ptr);
+    *offset = (size_t) ((uint8_t*) data - (uint8_t*) memory->mem.ptr);
      *size = b->length;

      pa_memblock_release(b);
diff --git a/src/pulsecore/privatemem.c b/src/pulsecore/privatemem.c
new file mode 100644
index 0000000..99df8c7
--- /dev/null
+++ b/src/pulsecore/privatemem.c
@@ -0,0 +1,78 @@
+/***
+    This file is part of PulseAudio.
+
+    Copyright 2006 Lennart Poettering
+    Copyright 2006 Pierre Ossman <[email protected]> for Cendio AB
+
+    PulseAudio is free software; you can redistribute it and/or modify
+    it under the terms of the GNU Lesser General Public License as
+    published by the Free Software Foundation; either version 2.1 of the
+    License, or (at your option) any later version.
+
+    PulseAudio is distributed in the hope that it will be useful, but
+    WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+    Lesser General Public License for more details.
+
+    You should have received a copy of the GNU Lesser General Public
+    License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
+***/
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <pulsecore/shm.h>
+#include <pulsecore/privatemem.h>
+#include <pulsecore/core-error.h>
+#include <pulse/xmalloc.h>
+
+#define MAX_MEM_SIZE    PA_MAX_SHM_SIZE
+
+int pa_privatemem_create(pa_privatemem *m, size_t size) {
+    pa_assert(m);
+    pa_assert(size > 0);
+    pa_assert(size <= MAX_MEM_SIZE);
+
+    /* Round up to make it page aligned */
+    size = PA_PAGE_ALIGN(size);
+
+    m->mem.size = size;
+
+#ifdef MAP_ANONYMOUS
+    if ((m->mem.ptr = mmap(NULL, m->mem.size, PROT_READ|PROT_WRITE, 
MAP_ANONYMOUS|MAP_PRIVATE, -1, (off_t) 0)) == MAP_FAILED) {
+        pa_log("mmap() failed: %s", pa_cstrerror(errno));
+        goto fail;
+    }
+#elif defined(HAVE_POSIX_MEMALIGN)
+    {
+        int r;
+
+        if ((r = posix_memalign(&m->mem.ptr, PA_PAGE_SIZE, size)) < 0) {
+            pa_log("posix_memalign() failed: %s", pa_cstrerror(r));
+            goto fail;
+        }
+    }
+#else
+    m->mem.ptr = pa_xmalloc(m->mem.size);
+#endif
+
+    return 0;
+fail:
+    return -1;
+}
+
+void pa_privatemem_free(pa_privatemem *m) {
+    pa_assert(m);
+    pa_assert(m->mem.ptr);
+    pa_assert(m->mem.size > 0);
+
+#ifdef MAP_ANONYMOUS
+    if (munmap(m->mem.ptr, m->mem.size) < 0)
+        pa_log("munmap() failed: %s", pa_cstrerror(errno));
+#elif defined(HAVE_POSIX_MEMALIGN)
+    free(m->mem.ptr);
+#else
+    pa_xfree(m->mem.ptr);
+#endif
+}
diff --git a/src/pulsecore/privatemem.h b/src/pulsecore/privatemem.h
new file mode 100644
index 0000000..9240ce0
--- /dev/null
+++ b/src/pulsecore/privatemem.h
@@ -0,0 +1,35 @@
+#ifndef foopulseprivatememhfoo
+#define foopulseprivatememhfoo
+
+/***
+  This file is part of PulseAudio.
+
+  Copyright 2015 Ahmed S. Darwish <[email protected]>
+
+  PulseAudio is free software; you can redistribute it and/or modify
+  it under the terms of the GNU Lesser General Public License as
+  published by the Free Software Foundation; either version 2.1 of the
+  License, or (at your option) any later version.
+
+  PulseAudio is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public
+  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <pulsecore/mem.h>
+
+/* Private memory for the mempools. This is usually used if posix SHM
+ * or Linux memfds are not available, or if we're explicitly asked
+ * not to use any form of shared memory. */
+typedef struct pa_privatemem {
+    pa_mem mem;             /* Parent; must be first */
+} pa_privatemem;
+
+int pa_privatemem_create(pa_privatemem *m, size_t size);
+void pa_privatemem_free(pa_privatemem *m);
+
+#endif
diff --git a/src/pulsecore/shm.c b/src/pulsecore/shm.c
index d613168..4c0ecf1 100644
--- a/src/pulsecore/shm.c
+++ b/src/pulsecore/shm.c
@@ -58,9 +58,6 @@
  #define MADV_REMOVE 9
  #endif

-/* 1 GiB at max */
-#define MAX_SHM_SIZE (PA_ALIGN(1024*1024*1024))
-
  #ifdef __linux__
  /* On Linux we know that the shared memory blocks are files in
   * /dev/shm. We can use that information to list all blocks and
@@ -100,15 +97,17 @@ static char *segment_name(char *fn, size_t l, unsigned id) 
{
  }
  #endif

-int pa_shm_create_rw(pa_shm *m, size_t size, bool shared, mode_t mode) {
+int pa_shm_create(pa_shm *m, pa_mem_type_t type, size_t size, mode_t mode) {
  #ifdef HAVE_SHM_OPEN
      char fn[32];
      int fd = -1;
  #endif
+    struct shm_marker *marker;

      pa_assert(m);
+    pa_assert(pa_mem_type_is_shared(type));
      pa_assert(size > 0);
-    pa_assert(size <= MAX_SHM_SIZE);
+    pa_assert(size <= PA_MAX_SHM_SIZE);
      pa_assert(!(mode & ~0777));
      pa_assert(mode >= 0600);

@@ -119,72 +118,42 @@ int pa_shm_create_rw(pa_shm *m, size_t size, bool shared, 
mode_t mode) {
      /* Round up to make it page aligned */
      size = PA_PAGE_ALIGN(size);

-    if (!shared) {
-        m->id = 0;
-        m->size = size;
-
-#ifdef MAP_ANONYMOUS
-        if ((m->ptr = mmap(NULL, m->size, PROT_READ|PROT_WRITE, 
MAP_ANONYMOUS|MAP_PRIVATE, -1, (off_t) 0)) == MAP_FAILED) {
-            pa_log("mmap() failed: %s", pa_cstrerror(errno));
-            goto fail;
-        }
-#elif defined(HAVE_POSIX_MEMALIGN)
-        {
-            int r;
-
-            if ((r = posix_memalign(&m->ptr, PA_PAGE_SIZE, size)) < 0) {
-                pa_log("posix_memalign() failed: %s", pa_cstrerror(r));
-                goto fail;
-            }
-        }
-#else
-        m->ptr = pa_xmalloc(m->size);
-#endif
-
-        m->do_unlink = false;
-
-    } else {
  #ifdef HAVE_SHM_OPEN
-        struct shm_marker *marker;
+    pa_random(&m->id, sizeof(m->id));
+    segment_name(fn, sizeof(fn), m->id);

-        pa_random(&m->id, sizeof(m->id));
-        segment_name(fn, sizeof(fn), m->id);
+    if ((fd = shm_open(fn, O_RDWR|O_CREAT|O_EXCL, mode)) < 0) {
+        pa_log("shm_open() failed: %s", pa_cstrerror(errno));
+        goto fail;
+    }

-        if ((fd = shm_open(fn, O_RDWR|O_CREAT|O_EXCL, mode)) < 0) {
-            pa_log("shm_open() failed: %s", pa_cstrerror(errno));
-            goto fail;
-        }
+    m->mem.size = size + SHM_MARKER_SIZE;

-        m->size = size + SHM_MARKER_SIZE;
-
-        if (ftruncate(fd, (off_t) m->size) < 0) {
-            pa_log("ftruncate() failed: %s", pa_cstrerror(errno));
-            goto fail;
-        }
+    if (ftruncate(fd, (off_t) m->mem.size) < 0) {
+        pa_log("ftruncate() failed: %s", pa_cstrerror(errno));
+        goto fail;
+    }

  #ifndef MAP_NORESERVE
  #define MAP_NORESERVE 0
  #endif

-        if ((m->ptr = mmap(NULL, PA_PAGE_ALIGN(m->size), PROT_READ|PROT_WRITE, 
MAP_SHARED|MAP_NORESERVE, fd, (off_t) 0)) == MAP_FAILED) {
-            pa_log("mmap() failed: %s", pa_cstrerror(errno));
-            goto fail;
-        }
-
-        /* We store our PID at the end of the shm block, so that we
-         * can check for dead shm segments later */
-        marker = (struct shm_marker*) ((uint8_t*) m->ptr + m->size - 
SHM_MARKER_SIZE);
-        pa_atomic_store(&marker->pid, (int) getpid());
-        pa_atomic_store(&marker->marker, SHM_MARKER);
-
-        pa_assert_se(pa_close(fd) == 0);
-        m->do_unlink = true;
-#else
+    if ((m->mem.ptr = mmap(NULL, PA_PAGE_ALIGN(m->mem.size), 
PROT_READ|PROT_WRITE, MAP_SHARED|MAP_NORESERVE, fd, (off_t) 0)) == MAP_FAILED) {
+        pa_log("mmap() failed: %s", pa_cstrerror(errno));
          goto fail;
-#endif
      }

-    m->shared = shared;
+    /* We store our PID at the end of the shm block, so that we
+     * can check for dead shm segments later */
+    marker = (struct shm_marker*) ((uint8_t*) m->mem.ptr + m->mem.size - 
SHM_MARKER_SIZE);
+    pa_atomic_store(&marker->pid, (int) getpid());
+    pa_atomic_store(&marker->marker, SHM_MARKER);
+
+    pa_assert_se(pa_close(fd) == 0);
+    m->do_unlink = true;
+#else
+    goto fail;
+#endif

      return 0;

@@ -202,40 +171,28 @@ fail:

  void pa_shm_free(pa_shm *m) {
      pa_assert(m);
-    pa_assert(m->ptr);
-    pa_assert(m->size > 0);
+    pa_assert(m->mem.ptr);
+    pa_assert(m->mem.size > 0);

  #ifdef MAP_FAILED
-    pa_assert(m->ptr != MAP_FAILED);
+    pa_assert(m->mem.ptr != MAP_FAILED);
  #endif

-    if (!m->shared) {
-#ifdef MAP_ANONYMOUS
-        if (munmap(m->ptr, m->size) < 0)
-            pa_log("munmap() failed: %s", pa_cstrerror(errno));
-#elif defined(HAVE_POSIX_MEMALIGN)
-        free(m->ptr);
-#else
-        pa_xfree(m->ptr);
-#endif
-    } else {
  #ifdef HAVE_SHM_OPEN
-        if (munmap(m->ptr, PA_PAGE_ALIGN(m->size)) < 0)
-            pa_log("munmap() failed: %s", pa_cstrerror(errno));
+    if (munmap(m->mem.ptr, PA_PAGE_ALIGN(m->mem.size)) < 0)
+        pa_log("munmap() failed: %s", pa_cstrerror(errno));

-        if (m->do_unlink) {
-            char fn[32];
+    if (m->do_unlink) {
+        char fn[32];

-            segment_name(fn, sizeof(fn), m->id);
-
-            if (shm_unlink(fn) < 0)
-                pa_log(" shm_unlink(%s) failed: %s", fn, pa_cstrerror(errno));
-        }
-#else
-        /* We shouldn't be here without shm support */
-        pa_assert_not_reached();
-#endif
+        segment_name(fn, sizeof(fn), m->id);
+        if (shm_unlink(fn) < 0)
+            pa_log(" shm_unlink(%s) failed: %s", fn, pa_cstrerror(errno));
      }
+#else
+    /* We shouldn't be here without shm support */
+    pa_assert_not_reached();
+#endif

      pa_zero(*m);
  }
@@ -245,19 +202,19 @@ void pa_shm_punch(pa_shm *m, size_t offset, size_t size) {
      size_t o;

      pa_assert(m);
-    pa_assert(m->ptr);
-    pa_assert(m->size > 0);
-    pa_assert(offset+size <= m->size);
+    pa_assert(m->mem.ptr);
+    pa_assert(m->mem.size > 0);
+    pa_assert(offset+size <= m->mem.size);

  #ifdef MAP_FAILED
-    pa_assert(m->ptr != MAP_FAILED);
+    pa_assert(m->mem.ptr != MAP_FAILED);
  #endif

      /* You're welcome to implement this as NOOP on systems that don't
       * support it */

      /* Align the pointer up to multiples of the page size */
-    ptr = (uint8_t*) m->ptr + offset;
+    ptr = (uint8_t*) m->mem.ptr + offset;
      o = (size_t) ((uint8_t*) ptr - (uint8_t*) PA_PAGE_ALIGN_PTR(ptr));

      if (o > 0) {
@@ -310,22 +267,21 @@ static int shm_attach(pa_shm *m, unsigned id, bool 
writable, bool for_cleanup) {
      }

      if (st.st_size <= 0 ||
-        st.st_size > (off_t) (MAX_SHM_SIZE+SHM_MARKER_SIZE) ||
+        st.st_size > (off_t) (PA_MAX_SHM_SIZE+SHM_MARKER_SIZE) ||
          PA_ALIGN((size_t) st.st_size) != (size_t) st.st_size) {
          pa_log("Invalid shared memory segment size");
          goto fail;
      }

-    m->size = (size_t) st.st_size;
+    m->mem.size = (size_t) st.st_size;

      prot = writable ? PROT_READ | PROT_WRITE : PROT_READ;
-    if ((m->ptr = mmap(NULL, PA_PAGE_ALIGN(m->size), prot, MAP_SHARED, fd, 
(off_t) 0)) == MAP_FAILED) {
+    if ((m->mem.ptr = mmap(NULL, PA_PAGE_ALIGN(m->mem.size), prot, MAP_SHARED, 
fd, (off_t) 0)) == MAP_FAILED) {
          pa_log("mmap() failed: %s", pa_cstrerror(errno));
          goto fail;
      }

      m->do_unlink = false;
-    m->shared = true;

      pa_assert_se(pa_close(fd) == 0);

@@ -382,12 +338,12 @@ int pa_shm_cleanup(void) {
          if (shm_attach(&seg, id, false, true) < 0)
              continue;

-        if (seg.size < SHM_MARKER_SIZE) {
+        if (seg.mem.size < SHM_MARKER_SIZE) {
              pa_shm_free(&seg);
              continue;
          }

-        m = (struct shm_marker*) ((uint8_t*) seg.ptr + seg.size - 
SHM_MARKER_SIZE);
+        m = (struct shm_marker*) ((uint8_t*) seg.mem.ptr + seg.mem.size - 
SHM_MARKER_SIZE);

          if (pa_atomic_load(&m->marker) != SHM_MARKER) {
              pa_shm_free(&seg);
diff --git a/src/pulsecore/shm.h b/src/pulsecore/shm.h
index d438961..887f516 100644
--- a/src/pulsecore/shm.h
+++ b/src/pulsecore/shm.h
@@ -23,16 +23,19 @@
  #include <sys/types.h>

  #include <pulsecore/macro.h>
+#include <pulsecore/mem.h>
+
+/* 1 GiB at max */
+#define PA_MAX_SHM_SIZE (PA_ALIGN(1024*1024*1024))

  typedef struct pa_shm {
+    pa_mem mem;             /* Parent; must be first */
+    pa_mem_type_t type;
      unsigned id;
-    void *ptr;
-    size_t size;
      bool do_unlink:1;
-    bool shared:1;
  } pa_shm;

-int pa_shm_create_rw(pa_shm *m, size_t size, bool shared, mode_t mode);
+int pa_shm_create(pa_shm *m, pa_mem_type_t type, size_t size, mode_t mode);
  int pa_shm_attach(pa_shm *m, unsigned id, bool writable);

  void pa_shm_punch(pa_shm *m, size_t offset, size_t size);

Regards,


--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to