All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] pulse: get rid of a number of assert()s
@ 2009-07-31 14:01 Lennart Poettering
  2009-07-31 14:48 ` Colin Guthrie
  2009-08-03 10:40 ` Takashi Iwai
  0 siblings, 2 replies; 5+ messages in thread
From: Lennart Poettering @ 2009-07-31 14:01 UTC (permalink / raw)
  To: ALSA Development Mailing List

Instead of hitting an assert when any of the plugin functions is called
in an invalid context we should return a clean error to make sure
programs are not unnecessarily aborted.

This should fix issues such as http://pulseaudio.org/ticket/595
---
 pulse/ctl_pulse.c |   35 +++++++++++++++++------
 pulse/pcm_pulse.c |   79 +++++++++++++++++++++++++++++++++++++++++------------
 pulse/pulse.c     |   34 ++++++++++++++++-------
 3 files changed, 111 insertions(+), 37 deletions(-)

diff --git a/pulse/ctl_pulse.c b/pulse/ctl_pulse.c
index c6cf9e2..1b057ef 100644
--- a/pulse/ctl_pulse.c
+++ b/pulse/ctl_pulse.c
@@ -125,8 +125,9 @@ static void event_cb(pa_context * c, pa_subscription_event_type_t t,
 	pa_operation *o;
 
 	assert(ctl);
-	assert(ctl->p);
-	assert(ctl->p->context);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return;
 
 	o = pa_context_get_sink_info_by_name(ctl->p->context, ctl->sink,
 					     sink_info_cb, ctl);
@@ -148,8 +149,9 @@ static int pulse_update_volume(snd_ctl_pulse_t * ctl)
 	pa_operation *o;
 
 	assert(ctl);
-	assert(ctl->p);
-	assert(ctl->p->context);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	o = pa_context_get_sink_info_by_name(ctl->p->context, ctl->sink,
 					     sink_info_cb, ctl);
@@ -203,6 +205,9 @@ static int pulse_elem_list(snd_ctl_ext_t * ext, unsigned int offset,
 
 	assert(ctl);
 
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
+
 	snd_ctl_elem_id_set_interface(id, SND_CTL_ELEM_IFACE_MIXER);
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
@@ -260,7 +265,9 @@ static int pulse_get_attribute(snd_ctl_ext_t * ext, snd_ctl_ext_key_t key,
 		return -EINVAL;
 
 	assert(ctl);
-	assert(ctl->p);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
@@ -311,7 +318,9 @@ static int pulse_read_integer(snd_ctl_ext_t * ext, snd_ctl_ext_key_t key,
 	pa_cvolume *vol = NULL;
 
 	assert(ctl);
-	assert(ctl->p);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
@@ -361,7 +370,9 @@ static int pulse_write_integer(snd_ctl_ext_t * ext, snd_ctl_ext_key_t key,
 	pa_cvolume *vol = NULL;
 
 	assert(ctl);
-	assert(ctl->p && ctl->p->context);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
@@ -465,6 +476,9 @@ static void pulse_subscribe_events(snd_ctl_ext_t * ext, int subscribe)
 
 	assert(ctl);
 
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return;
+
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
 	ctl->subscribed = !!(subscribe & SND_CTL_EVENT_MASK_VALUE);
@@ -481,6 +495,9 @@ static int pulse_read_event(snd_ctl_ext_t * ext, snd_ctl_elem_id_t * id,
 
 	assert(ctl);
 
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
+
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
 	if (!ctl->updated || !ctl->subscribed)
@@ -525,8 +542,8 @@ static int pulse_ctl_poll_revents(snd_ctl_ext_t * ext, struct pollfd *pfd,
 	snd_ctl_pulse_t *ctl = ext->private_data;
 	int err = 0;
 
-	assert(ctl);
-	assert(ctl->p);
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
diff --git a/pulse/pcm_pulse.c b/pulse/pcm_pulse.c
index c276839..24347f9 100644
--- a/pulse/pcm_pulse.c
+++ b/pulse/pcm_pulse.c
@@ -106,6 +106,9 @@ static int update_active(snd_pcm_pulse_t *pcm) {
 
 	assert(pcm);
 
+	if (!pcm->p)
+		return -EBADFD;
+
 	ret = check_active(pcm);
 	if (ret < 0)
 		return ret;
@@ -125,7 +128,9 @@ static int pulse_start(snd_pcm_ioplug_t * io)
 	int err = 0, err_o = 0, err_u = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -174,7 +179,9 @@ static int pulse_stop(snd_pcm_ioplug_t * io)
 	int err = 0, err_o = 0, err_u = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -224,7 +231,9 @@ static int pulse_drain(snd_pcm_ioplug_t * io)
 	int err = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -259,7 +268,9 @@ static snd_pcm_sframes_t pulse_pointer(snd_pcm_ioplug_t * io)
 	snd_pcm_sframes_t ret = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	if (io->state == SND_PCM_STATE_XRUN)
 		return -EPIPE;
@@ -269,7 +280,10 @@ static snd_pcm_sframes_t pulse_pointer(snd_pcm_ioplug_t * io)
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
-	assert(pcm->stream);
+	if (!pcm->stream) {
+		ret = -EBADFD;
+		goto finish;
+	}
 
 	ret = pulse_check_connection(pcm->p);
 	if (ret < 0)
@@ -305,11 +319,16 @@ static int pulse_delay(snd_pcm_ioplug_t * io, snd_pcm_sframes_t * delayp)
 	pa_usec_t lat = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
-	assert(pcm->stream);
+	if (!pcm->stream) {
+		err = -EBADFD;
+		goto finish;
+	}
 
 	for (;;) {
 		err = pulse_check_connection(pcm->p);
@@ -354,11 +373,16 @@ static snd_pcm_sframes_t pulse_write(snd_pcm_ioplug_t * io,
 	snd_pcm_sframes_t ret = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
-	assert(pcm->stream);
+	if (!pcm->stream) {
+		ret = -EBADFD;
+		goto finish;
+	}
 
 	ret = pulse_check_connection(pcm->p);
 	if (ret < 0)
@@ -409,11 +433,16 @@ static snd_pcm_sframes_t pulse_read(snd_pcm_ioplug_t * io,
 	snd_pcm_sframes_t ret = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
-	assert(pcm->stream);
+	if (!pcm->stream) {
+		ret = -EBADFD;
+		goto finish;
+	}
 
 	ret = pulse_check_connection(pcm->p);
 	if (ret < 0)
@@ -480,7 +509,9 @@ static void stream_request_cb(pa_stream * p, size_t length, void *userdata)
 	snd_pcm_pulse_t *pcm = userdata;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return;
 
 	update_active(pcm);
 }
@@ -490,7 +521,9 @@ static void stream_underrun_cb(pa_stream * p, void *userdata)
 	snd_pcm_pulse_t *pcm = userdata;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return;
 
 	pcm->underrun = 1;
 }
@@ -499,7 +532,9 @@ static void stream_latency_cb(pa_stream *p, void *userdata) {
 	snd_pcm_pulse_t *pcm = userdata;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return;
 
 	pa_threaded_mainloop_signal(pcm->p->mainloop, 0);
 }
@@ -512,7 +547,9 @@ static int pulse_pcm_poll_revents(snd_pcm_ioplug_t * io,
 	snd_pcm_pulse_t *pcm = io->private_data;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -541,7 +578,9 @@ static int pulse_prepare(snd_pcm_ioplug_t * io)
 	unsigned c, d;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -645,7 +684,9 @@ static int pulse_hw_params(snd_pcm_ioplug_t * io,
 	int err = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -745,7 +786,9 @@ static int pulse_pause(snd_pcm_ioplug_t * io, int enable)
 	int err = 0;
 
 	assert (pcm);
-	assert (pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
diff --git a/pulse/pulse.c b/pulse/pulse.c
index 3940238..95d8dde 100644
--- a/pulse/pulse.c
+++ b/pulse/pulse.c
@@ -32,8 +32,9 @@ int pulse_check_connection(snd_pulse_t * p)
 	pa_context_state_t state;
 
 	assert(p);
-	assert(p->context);
-	assert(p->mainloop);
+
+	if (!p->context || !p->mainloop)
+		return -EBADFD;
 
 	state = pa_context_get_state(p->context);
 
@@ -77,8 +78,12 @@ int pulse_wait_operation(snd_pulse_t * p, pa_operation * o)
 {
 	assert(p);
 	assert(o);
-	assert(p->state == PULSE_STATE_READY);
-	assert(p->mainloop);
+
+	if (p->state != PULSE_STATE_READY)
+		return -EBADFD;
+
+	if (!p->mainloop)
+		return -EBADFD;
 
 	for (;;) {
 		int err;
@@ -103,8 +108,12 @@ int pulse_wait_stream_state(snd_pulse_t * p, pa_stream * stream,
 
 	assert(p);
 	assert(stream);
-	assert(p->state == PULSE_STATE_READY);
-	assert(p->mainloop);
+
+	if (p->state != PULSE_STATE_READY)
+		return -EBADFD;
+
+	if (!p->mainloop)
+		return -EBADFD;
 
 	for (;;) {
 		int err;
@@ -197,7 +206,9 @@ snd_pulse_t *pulse_new(void)
 
 	p->context =
 	    pa_context_new(pa_threaded_mainloop_get_api(p->mainloop), buf);
-	assert(p->context);
+
+	if (!p->context)
+		goto fail;
 
 	pa_context_set_state_callback(p->context, context_state_cb, p);
 
@@ -246,9 +257,12 @@ int pulse_connect(snd_pulse_t * p, const char *server)
 	int err;
 
 	assert(p);
-	assert(p->context);
-	assert(p->mainloop);
-	assert(p->state == PULSE_STATE_INIT);
+
+	if (!p->context || !p->mainloop)
+		return -EBADFD;
+
+	if (p->state != PULSE_STATE_INIT)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(p->mainloop);
 
-- 
1.6.3.3



Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net         ICQ# 11060553
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/4] pulse: get rid of a number of assert()s
  2009-07-31 14:01 [PATCH 1/4] pulse: get rid of a number of assert()s Lennart Poettering
@ 2009-07-31 14:48 ` Colin Guthrie
  2009-07-31 15:10   ` Lennart Poettering
  2009-07-31 15:15   ` Lennart Poettering
  2009-08-03 10:40 ` Takashi Iwai
  1 sibling, 2 replies; 5+ messages in thread
From: Colin Guthrie @ 2009-07-31 14:48 UTC (permalink / raw)
  To: alsa-devel

'Twas brillig, and Lennart Poettering at 31/07/09 15:01 did gyre and gimble:
> Instead of hitting an assert when any of the plugin functions is called
> in an invalid context we should return a clean error to make sure
> programs are not unnecessarily aborted.
> 
> This should fix issues such as http://pulseaudio.org/ticket/595
> ---
>  pulse/ctl_pulse.c |   35 +++++++++++++++++------
>  pulse/pcm_pulse.c |   79 +++++++++++++++++++++++++++++++++++++++++------------
>  pulse/pulse.c     |   34 ++++++++++++++++-------
>  3 files changed, 111 insertions(+), 37 deletions(-)
> 
> diff --git a/pulse/ctl_pulse.c b/pulse/ctl_pulse.c
> index c6cf9e2..1b057ef 100644
> --- a/pulse/ctl_pulse.c
> +++ b/pulse/ctl_pulse.c

> @@ -525,8 +542,8 @@ static int pulse_ctl_poll_revents(snd_ctl_ext_t * ext, struct pollfd *pfd,
>  	snd_ctl_pulse_t *ctl = ext->private_data;
>  	int err = 0;
>  
> -	assert(ctl);
> -	assert(ctl->p);
> +	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
> +		return -EBADFD;
>  
>  	pa_threaded_mainloop_lock(ctl->p->mainloop);
>  


Just a smidge overzealous in this one (only one I spotted).


Couldn't find these commits in your alsa-plugins clone at 
git://git.0pointer.de/alsa-plugins.git so couldn't create a followup 
patch so here it is inline (against 1.2.20 so line numbers may be off):

--- alsa-plugins-1.0.20/pulse/ctl_pulse.c~	2009-07-31 15:39:22.000000000 
+0100
+++ alsa-plugins-1.0.20/pulse/ctl_pulse.c	2009-07-31 15:47:32.000000000 
+0100
@@ -542,6 +542,8 @@
  	snd_ctl_pulse_t *ctl = ext->private_data;
  	int err = 0;

+	assert(ctl);
+
  	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
  		return -EBADFD;




Col.




-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
   Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
   Mandriva Linux Contributor [http://www.mandriva.com/]
   PulseAudio Hacker [http://www.pulseaudio.org/]
   Trac Hacker [http://trac.edgewall.org/]

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/4] pulse: get rid of a number of assert()s
  2009-07-31 14:48 ` Colin Guthrie
@ 2009-07-31 15:10   ` Lennart Poettering
  2009-07-31 15:15   ` Lennart Poettering
  1 sibling, 0 replies; 5+ messages in thread
From: Lennart Poettering @ 2009-07-31 15:10 UTC (permalink / raw)
  To: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Fri, 31.07.09 15:48, Colin Guthrie (gmane@colin.guthr.ie) wrote:

> Couldn't find these commits in your alsa-plugins clone at 
> git://git.0pointer.de/alsa-plugins.git so couldn't create a followup 
> patch so here it is inline (against 1.2.20 so line numbers may be off):
> 
> --- alsa-plugins-1.0.20/pulse/ctl_pulse.c~	2009-07-31 15:39:22.000000000 
> +0100
> +++ alsa-plugins-1.0.20/pulse/ctl_pulse.c	2009-07-31 15:47:32.000000000 
> +0100
> @@ -542,6 +542,8 @@
>   	snd_ctl_pulse_t *ctl = ext->private_data;
>   	int err = 0;
> 
> +	assert(ctl);
> +
>   	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
>   		return -EBADFD;
> 

See attachment for a version that incorporates this mini fix.

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net         ICQ# 11060553
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

[-- Attachment #2: 0001-pulse-get-rid-of-a-number-of-assert-s.patch --]
[-- Type: text/plain, Size: 9234 bytes --]

>From 5ba1cc327b329ae518c3413e65b44945d5d8f919 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 31 Jul 2009 15:25:44 +0200
Subject: [PATCH] pulse: get rid of a number of assert()s

Instead of hitting an assert when any of the plugin functions is called
in an invalid context we should return a clean error to make sure
programs are not unnecessarily aborted.

This should fix issues such as http://pulseaudio.org/ticket/595
---
 pulse/ctl_pulse.c |   35 ++++++++++++++++++-----
 pulse/pcm_pulse.c |   79 +++++++++++++++++++++++++++++++++++++++++------------
 pulse/pulse.c     |   34 ++++++++++++++++-------
 3 files changed, 112 insertions(+), 36 deletions(-)

diff --git a/pulse/ctl_pulse.c b/pulse/ctl_pulse.c
index c6cf9e2..2caa29b 100644
--- a/pulse/ctl_pulse.c
+++ b/pulse/ctl_pulse.c
@@ -125,8 +125,9 @@ static void event_cb(pa_context * c, pa_subscription_event_type_t t,
 	pa_operation *o;
 
 	assert(ctl);
-	assert(ctl->p);
-	assert(ctl->p->context);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return;
 
 	o = pa_context_get_sink_info_by_name(ctl->p->context, ctl->sink,
 					     sink_info_cb, ctl);
@@ -148,8 +149,9 @@ static int pulse_update_volume(snd_ctl_pulse_t * ctl)
 	pa_operation *o;
 
 	assert(ctl);
-	assert(ctl->p);
-	assert(ctl->p->context);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	o = pa_context_get_sink_info_by_name(ctl->p->context, ctl->sink,
 					     sink_info_cb, ctl);
@@ -203,6 +205,9 @@ static int pulse_elem_list(snd_ctl_ext_t * ext, unsigned int offset,
 
 	assert(ctl);
 
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
+
 	snd_ctl_elem_id_set_interface(id, SND_CTL_ELEM_IFACE_MIXER);
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
@@ -260,7 +265,9 @@ static int pulse_get_attribute(snd_ctl_ext_t * ext, snd_ctl_ext_key_t key,
 		return -EINVAL;
 
 	assert(ctl);
-	assert(ctl->p);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
@@ -311,7 +318,9 @@ static int pulse_read_integer(snd_ctl_ext_t * ext, snd_ctl_ext_key_t key,
 	pa_cvolume *vol = NULL;
 
 	assert(ctl);
-	assert(ctl->p);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
@@ -361,7 +370,9 @@ static int pulse_write_integer(snd_ctl_ext_t * ext, snd_ctl_ext_key_t key,
 	pa_cvolume *vol = NULL;
 
 	assert(ctl);
-	assert(ctl->p && ctl->p->context);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
@@ -465,6 +476,9 @@ static void pulse_subscribe_events(snd_ctl_ext_t * ext, int subscribe)
 
 	assert(ctl);
 
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return;
+
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
 	ctl->subscribed = !!(subscribe & SND_CTL_EVENT_MASK_VALUE);
@@ -481,6 +495,9 @@ static int pulse_read_event(snd_ctl_ext_t * ext, snd_ctl_elem_id_t * id,
 
 	assert(ctl);
 
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
+
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
 	if (!ctl->updated || !ctl->subscribed)
@@ -526,7 +543,9 @@ static int pulse_ctl_poll_revents(snd_ctl_ext_t * ext, struct pollfd *pfd,
 	int err = 0;
 
 	assert(ctl);
-	assert(ctl->p);
+
+	if (!ctl->p || !ctl->p->mainloop || !ctl->p->context)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(ctl->p->mainloop);
 
diff --git a/pulse/pcm_pulse.c b/pulse/pcm_pulse.c
index c276839..24347f9 100644
--- a/pulse/pcm_pulse.c
+++ b/pulse/pcm_pulse.c
@@ -106,6 +106,9 @@ static int update_active(snd_pcm_pulse_t *pcm) {
 
 	assert(pcm);
 
+	if (!pcm->p)
+		return -EBADFD;
+
 	ret = check_active(pcm);
 	if (ret < 0)
 		return ret;
@@ -125,7 +128,9 @@ static int pulse_start(snd_pcm_ioplug_t * io)
 	int err = 0, err_o = 0, err_u = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -174,7 +179,9 @@ static int pulse_stop(snd_pcm_ioplug_t * io)
 	int err = 0, err_o = 0, err_u = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -224,7 +231,9 @@ static int pulse_drain(snd_pcm_ioplug_t * io)
 	int err = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -259,7 +268,9 @@ static snd_pcm_sframes_t pulse_pointer(snd_pcm_ioplug_t * io)
 	snd_pcm_sframes_t ret = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	if (io->state == SND_PCM_STATE_XRUN)
 		return -EPIPE;
@@ -269,7 +280,10 @@ static snd_pcm_sframes_t pulse_pointer(snd_pcm_ioplug_t * io)
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
-	assert(pcm->stream);
+	if (!pcm->stream) {
+		ret = -EBADFD;
+		goto finish;
+	}
 
 	ret = pulse_check_connection(pcm->p);
 	if (ret < 0)
@@ -305,11 +319,16 @@ static int pulse_delay(snd_pcm_ioplug_t * io, snd_pcm_sframes_t * delayp)
 	pa_usec_t lat = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
-	assert(pcm->stream);
+	if (!pcm->stream) {
+		err = -EBADFD;
+		goto finish;
+	}
 
 	for (;;) {
 		err = pulse_check_connection(pcm->p);
@@ -354,11 +373,16 @@ static snd_pcm_sframes_t pulse_write(snd_pcm_ioplug_t * io,
 	snd_pcm_sframes_t ret = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
-	assert(pcm->stream);
+	if (!pcm->stream) {
+		ret = -EBADFD;
+		goto finish;
+	}
 
 	ret = pulse_check_connection(pcm->p);
 	if (ret < 0)
@@ -409,11 +433,16 @@ static snd_pcm_sframes_t pulse_read(snd_pcm_ioplug_t * io,
 	snd_pcm_sframes_t ret = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
-	assert(pcm->stream);
+	if (!pcm->stream) {
+		ret = -EBADFD;
+		goto finish;
+	}
 
 	ret = pulse_check_connection(pcm->p);
 	if (ret < 0)
@@ -480,7 +509,9 @@ static void stream_request_cb(pa_stream * p, size_t length, void *userdata)
 	snd_pcm_pulse_t *pcm = userdata;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return;
 
 	update_active(pcm);
 }
@@ -490,7 +521,9 @@ static void stream_underrun_cb(pa_stream * p, void *userdata)
 	snd_pcm_pulse_t *pcm = userdata;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return;
 
 	pcm->underrun = 1;
 }
@@ -499,7 +532,9 @@ static void stream_latency_cb(pa_stream *p, void *userdata) {
 	snd_pcm_pulse_t *pcm = userdata;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return;
 
 	pa_threaded_mainloop_signal(pcm->p->mainloop, 0);
 }
@@ -512,7 +547,9 @@ static int pulse_pcm_poll_revents(snd_pcm_ioplug_t * io,
 	snd_pcm_pulse_t *pcm = io->private_data;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -541,7 +578,9 @@ static int pulse_prepare(snd_pcm_ioplug_t * io)
 	unsigned c, d;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -645,7 +684,9 @@ static int pulse_hw_params(snd_pcm_ioplug_t * io,
 	int err = 0;
 
 	assert(pcm);
-	assert(pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
@@ -745,7 +786,9 @@ static int pulse_pause(snd_pcm_ioplug_t * io, int enable)
 	int err = 0;
 
 	assert (pcm);
-	assert (pcm->p);
+
+	if (!pcm->p)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(pcm->p->mainloop);
 
diff --git a/pulse/pulse.c b/pulse/pulse.c
index 3940238..95d8dde 100644
--- a/pulse/pulse.c
+++ b/pulse/pulse.c
@@ -32,8 +32,9 @@ int pulse_check_connection(snd_pulse_t * p)
 	pa_context_state_t state;
 
 	assert(p);
-	assert(p->context);
-	assert(p->mainloop);
+
+	if (!p->context || !p->mainloop)
+		return -EBADFD;
 
 	state = pa_context_get_state(p->context);
 
@@ -77,8 +78,12 @@ int pulse_wait_operation(snd_pulse_t * p, pa_operation * o)
 {
 	assert(p);
 	assert(o);
-	assert(p->state == PULSE_STATE_READY);
-	assert(p->mainloop);
+
+	if (p->state != PULSE_STATE_READY)
+		return -EBADFD;
+
+	if (!p->mainloop)
+		return -EBADFD;
 
 	for (;;) {
 		int err;
@@ -103,8 +108,12 @@ int pulse_wait_stream_state(snd_pulse_t * p, pa_stream * stream,
 
 	assert(p);
 	assert(stream);
-	assert(p->state == PULSE_STATE_READY);
-	assert(p->mainloop);
+
+	if (p->state != PULSE_STATE_READY)
+		return -EBADFD;
+
+	if (!p->mainloop)
+		return -EBADFD;
 
 	for (;;) {
 		int err;
@@ -197,7 +206,9 @@ snd_pulse_t *pulse_new(void)
 
 	p->context =
 	    pa_context_new(pa_threaded_mainloop_get_api(p->mainloop), buf);
-	assert(p->context);
+
+	if (!p->context)
+		goto fail;
 
 	pa_context_set_state_callback(p->context, context_state_cb, p);
 
@@ -246,9 +257,12 @@ int pulse_connect(snd_pulse_t * p, const char *server)
 	int err;
 
 	assert(p);
-	assert(p->context);
-	assert(p->mainloop);
-	assert(p->state == PULSE_STATE_INIT);
+
+	if (!p->context || !p->mainloop)
+		return -EBADFD;
+
+	if (p->state != PULSE_STATE_INIT)
+		return -EBADFD;
 
 	pa_threaded_mainloop_lock(p->mainloop);
 
-- 
1.6.3.3


[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/4] pulse: get rid of a number of assert()s
  2009-07-31 14:48 ` Colin Guthrie
  2009-07-31 15:10   ` Lennart Poettering
@ 2009-07-31 15:15   ` Lennart Poettering
  1 sibling, 0 replies; 5+ messages in thread
From: Lennart Poettering @ 2009-07-31 15:15 UTC (permalink / raw)
  To: alsa-devel

On Fri, 31.07.09 15:48, Colin Guthrie (gmane@colin.guthr.ie) wrote:

> Couldn't find these commits in your alsa-plugins clone at 
> git://git.0pointer.de/alsa-plugins.git so couldn't create a followup 
> patch so here it is inline (against 1.2.20 so line numbers may be off):

My git tree is now updated.

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net         ICQ# 11060553
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/4] pulse: get rid of a number of assert()s
  2009-07-31 14:01 [PATCH 1/4] pulse: get rid of a number of assert()s Lennart Poettering
  2009-07-31 14:48 ` Colin Guthrie
@ 2009-08-03 10:40 ` Takashi Iwai
  1 sibling, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2009-08-03 10:40 UTC (permalink / raw)
  To: Lennart Poettering; +Cc: ALSA Development Mailing List

At Fri, 31 Jul 2009 16:01:33 +0200,
Lennart Poettering wrote:
> 
> Instead of hitting an assert when any of the plugin functions is called
> in an invalid context we should return a clean error to make sure
> programs are not unnecessarily aborted.
> 
> This should fix issues such as http://pulseaudio.org/ticket/595

Applied all four patches now.  Thanks!


Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-08-03 10:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-31 14:01 [PATCH 1/4] pulse: get rid of a number of assert()s Lennart Poettering
2009-07-31 14:48 ` Colin Guthrie
2009-07-31 15:10   ` Lennart Poettering
2009-07-31 15:15   ` Lennart Poettering
2009-08-03 10:40 ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.