Good catch. However the fail label could just call pa_srbchannel_free
instead of copy-pasting, unless there is a good reason not to?
On 2014-08-13 10:15, Peter Meerwald wrote:
> pa_fdsem_open_shm() returns NULL when HAVE_SYS_EVENTFD_H is #undefined
>
> pa_srbchannel_new() and pa_srbchannel_new_from_template() depend on
> pa_fdsem_open_shm() and shall properly cleanup stuff, and return NULL as well;
> otherwise, function pa_fdsem_get() will assert:
>
> Assertion 'f' failed at pulsecore/fdsem.c:284, function pa_fdsem_get(). Aborting.
>
> Debian/kFreeBSD doesn't HAVE_SYS_EVENTFD_H
>
> Signed-off-by: Peter Meerwald <pmeerw at pmeerw.net>
> Cc: David Henningsson <david.henningsson at canonical.com>
> ---
> src/pulsecore/srbchannel.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/src/pulsecore/srbchannel.c b/src/pulsecore/srbchannel.c
> index 87eeae0..9ac5b1d 100644
> --- a/src/pulsecore/srbchannel.c
> +++ b/src/pulsecore/srbchannel.c
> @@ -211,7 +211,12 @@ pa_srbchannel* pa_srbchannel_new(pa_mainloop_api *m, pa_mempool *p) {
> sr->rb_write.count = &srh->write_count;
>
> sr->sem_read = pa_fdsem_new_shm(&srh->read_semdata);
> + if (!sr->sem_read)
> + goto fail;
> +
> sr->sem_write = pa_fdsem_new_shm(&srh->write_semdata);
> + if (!sr->sem_write)
> + goto fail;
>
> readfd = pa_fdsem_get(sr->sem_read);
> #ifdef DEBUG_SRBCHANNEL
> @@ -221,6 +226,19 @@ pa_srbchannel* pa_srbchannel_new(pa_mainloop_api *m, pa_mempool *p) {
> m->io_enable(sr->read_event, PA_IO_EVENT_INPUT);
>
> return sr;
> +
> +fail:
> + if (sr->sem_write)
> + pa_fdsem_free(sr->sem_write);
> + if (sr->sem_read)
> + pa_fdsem_free(sr->sem_read);
> + if (sr->memblock) {
> + pa_memblock_release(sr->memblock);
> + pa_memblock_unref(sr->memblock);
> + }
> + pa_xfree(sr);
> +
> + return NULL;
> }
>
> static void pa_srbchannel_swap(pa_srbchannel *sr) {
> @@ -249,7 +267,12 @@ pa_srbchannel* pa_srbchannel_new_from_template(pa_mainloop_api *m, pa_srbchannel
> sr->rb_write.memory = (uint8_t*) srh + srh->writebuf_offset;
>
> sr->sem_read = pa_fdsem_open_shm(&srh->read_semdata, t->readfd);
> + if (!sr->sem_read)
> + goto fail;
> +
> sr->sem_write = pa_fdsem_open_shm(&srh->write_semdata, t->writefd);
> + if (!sr->sem_write)
> + goto fail;
>
> pa_srbchannel_swap(sr);
> temp = t->readfd; t->readfd = t->writefd; t->writefd = temp;
> @@ -261,6 +284,19 @@ pa_srbchannel* pa_srbchannel_new_from_template(pa_mainloop_api *m, pa_srbchannel
> m->io_enable(sr->read_event, PA_IO_EVENT_INPUT);
>
> return sr;
> +
> +fail:
> + if (sr->sem_write)
> + pa_fdsem_free(sr->sem_write);
> + if (sr->sem_read)
> + pa_fdsem_free(sr->sem_read);
> + if (sr->memblock) {
> + pa_memblock_release(sr->memblock);
> + pa_memblock_unref(sr->memblock);
> + }
> + pa_xfree(sr);
> +
> + return NULL;
> }
>
> void pa_srbchannel_export(pa_srbchannel *sr, pa_srbchannel_template *t) {
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
