[pulseaudio-discuss] [PATCH 3/8] srbchannel: Cleanup when pa_fdsem_open_shm() fails

[David Henningsson]

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